Re: [PATCH RFT 4/5] drm/msm: Always use component model for standalone GPU

[Akhil P Oommen <akhilpo@oss.qualcomm.com>]

On 5/13/2026 5:13 PM, Dmitry Baryshkov wrote:
> On Wed, May 13, 2026 at 02:22:59AM +0530, Akhil P Oommen wrote:
>> When separate_gpu_kms=1 (or the device is compatible with "amd,imageon"),
>> msm_gpu_no_components() evaluates true and adreno_probe() bypasses the
>> component framework by calling msm_gpu_probe() directly. In this case,
>> we skip creating components and directly bind the GPU.
>>
>> That shortcut makes it impossible to add a second sibling on the GPU
>> master without introducing ad-hoc plumbing. To prepare for adding the
>> GMU as a peer component on this master, turn the GPU pdev into both a
>> component master and the sole component of itself in this path. A follow
>> up patch will introduce GMU as a component device.
>>
>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>> ---
>>  drivers/gpu/drm/msm/adreno/adreno_device.c | 20 ++++++++++++-----
>>  drivers/gpu/drm/msm/msm_drv.c              | 35 +++++++++++++++++++++++++-----
>>  drivers/gpu/drm/msm/msm_drv.h              |  6 ++---
>>  3 files changed, 45 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> index 66953e551d86..67686424f3a1 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> @@ -271,18 +271,26 @@ static const struct component_ops a3xx_ops = {
>>  
>>  static int adreno_probe(struct platform_device *pdev)
>>  {
>> -	if (msm_gpu_use_separate_drm_dev(pdev))
>> -		return msm_gpu_probe(pdev, &a3xx_ops);
>> +	int ret;
>>  
>> -	return component_add(&pdev->dev, &a3xx_ops);
>> +	if (msm_gpu_use_separate_drm_dev(pdev)) {
>> +		ret = msm_gpu_probe(pdev);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = component_add(&pdev->dev, &a3xx_ops);
>> +	if (ret && msm_gpu_use_separate_drm_dev(pdev))
>> +		msm_gpu_remove(pdev);
> 
> What about making it more clear:
> 
> 	if (!msm_gpu_use_separate_drm_dev(pdev))
> 		return component_add(&pdev->dev, &a3xx_ops);
> 
> 	ret = msm_gpu_probe(pdev);
> 	if (ret)
> 		return ret;
> 
> 	ret = component_add(&pdev->dev, &a3xx_ops);
> 
> 	if (ret)
> 		msm_gpu_remove(pdev);
> 
> 	return ret;

Ack. this is more readable.

> 
> However with this patch in place, maybe it's easier to use the main
> msm_drv_probe()? It would need some fixes to handle !kms case as the
> GPU-only, but it looks very similar to your new functions.
>

Agree. Will do this in a separate patch in the next rev.

> 
>> +
>> +	return ret;
>>  }
>>  
>>  static void adreno_remove(struct platform_device *pdev)
>>  {
>> +	component_del(&pdev->dev, &a3xx_ops);
>>  	if (msm_gpu_use_separate_drm_dev(pdev))
>> -		msm_gpu_remove(pdev, &a3xx_ops);
>> -	else
>> -		component_del(&pdev->dev, &a3xx_ops);
>> +		msm_gpu_remove(pdev);
>>  }
>>  
>>  static void adreno_shutdown(struct platform_device *pdev)
>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
>> index b61deafd02c3..af5aa7ff6179 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.c
>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>> @@ -1097,10 +1097,25 @@ int msm_drv_probe(struct device *master_dev,
>>  	return 0;
>>  }
>>  
>> -int msm_gpu_probe(struct platform_device *pdev,
>> -		  const struct component_ops *ops)
>> +static int msm_gpu_drm_bind(struct device *dev)
>> +{
>> +	return msm_drm_init(dev, &msm_gpu_driver, NULL);
> 
> With this patch in place, we can remove the ops argument from
> msm_drm_init() and msm_drm_uninit().

I didn't notice this. Will drop the unused arg. Thanks.

-Akhil.

> 
>> +}
>> +
>> +static void msm_gpu_drm_unbind(struct device *dev)
>> +{
>> +	msm_drm_uninit(dev, NULL);
>> +}
>> +
>> +static const struct component_master_ops msm_gpu_drm_ops = {
>> +	.bind = msm_gpu_drm_bind,
>> +	.unbind = msm_gpu_drm_unbind,
>> +};
>> +
>> +int msm_gpu_probe(struct platform_device *pdev)
>>  {
>>  	struct msm_drm_private *priv;
>> +	struct component_match *match = NULL;
>>  	int ret;
>>  
>>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> @@ -1116,13 +1131,21 @@ int msm_gpu_probe(struct platform_device *pdev,
>>  	if (ret)
>>  		return ret;
>>  
>> -	return msm_drm_init(&pdev->dev, &msm_gpu_driver, ops);
>> +	/*
>> +	 * The GPU pdev acts as both the component master and the sole
>> +	 * component (added by adreno_probe()). Future patches add the
>> +	 * GMU node as a second component on this same master.
>> +	 */
>> +	drm_of_component_match_add(&pdev->dev, &match,
>> +				   component_compare_of, pdev->dev.of_node);
>> +
>> +	return component_master_add_with_match(&pdev->dev, &msm_gpu_drm_ops,
>> +					       match);
>>  }
>>  
>> -void msm_gpu_remove(struct platform_device *pdev,
>> -		    const struct component_ops *ops)
>> +void msm_gpu_remove(struct platform_device *pdev)
>>  {
>> -	msm_drm_uninit(&pdev->dev, ops);
>> +	component_master_del(&pdev->dev, &msm_gpu_drm_ops);
>>  }
>>  
>>  static int __init msm_drm_register(void)
>> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
>> index 6fcb696ceb7c..6264ff27496f 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.h
>> +++ b/drivers/gpu/drm/msm/msm_drv.h
>> @@ -544,10 +544,8 @@ extern const struct component_master_ops msm_drm_ops;
>>  int msm_kms_pm_prepare(struct device *dev);
>>  void msm_kms_pm_complete(struct device *dev);
>>  
>> -int msm_gpu_probe(struct platform_device *pdev,
>> -		  const struct component_ops *ops);
>> -void msm_gpu_remove(struct platform_device *pdev,
>> -		    const struct component_ops *ops);
>> +int msm_gpu_probe(struct platform_device *pdev);
>> +void msm_gpu_remove(struct platform_device *pdev);
>>  int msm_drv_probe(struct device *dev,
>>  	int (*kms_init)(struct drm_device *dev),
>>  	struct msm_kms *kms);
>>
>> -- 
>> 2.51.0
>>
>