diff mbox series

[v1,4/5] drm/msm/dp: move relevant dp initialization code from bind() to probe()

Message ID 1688773943-3887-5-git-send-email-quic_khsieh@quicinc.com (mailing list archive)
State Superseded
Headers show
Series incorporate pm runtime framework and eDP clean up | expand

Commit Message

Kuogee Hsieh July 7, 2023, 11:52 p.m. UTC
In preparation of moving edp of_dp_aux_populate_bus() to
dp_display_probe(), move dp_display_request_irq(),
dp->parser->parse() and dp_power_client_init() to dp_display_probe()
too.

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 48 +++++++++++++++++--------------------
 drivers/gpu/drm/msm/dp/dp_display.h |  1 -
 2 files changed, 22 insertions(+), 27 deletions(-)

Comments

Dmitry Baryshkov July 8, 2023, 12:11 a.m. UTC | #1
On 08/07/2023 02:52, Kuogee Hsieh wrote:
> In preparation of moving edp of_dp_aux_populate_bus() to
> dp_display_probe(), move dp_display_request_irq(),
> dp->parser->parse() and dp_power_client_init() to dp_display_probe()
> too.
> 
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 48 +++++++++++++++++--------------------
>   drivers/gpu/drm/msm/dp/dp_display.h |  1 -
>   2 files changed, 22 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 44580c2..185f1eb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -290,12 +290,6 @@ static int dp_display_bind(struct device *dev, struct device *master,
>   		goto end;
>   	}
>   
> -	rc = dp_power_client_init(dp->power);
> -	if (rc) {
> -		DRM_ERROR("Power client create failed\n");
> -		goto end;
> -	}
> -
>   	rc = dp_register_audio_driver(dev, dp->audio);
>   	if (rc) {
>   		DRM_ERROR("Audio registration Dp failed\n");
> @@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
>   		goto error;
>   	}
>   
> +	rc = dp->parser->parse(dp->parser);
> +	if (rc) {
> +		DRM_ERROR("device tree parsing failed\n");
> +		goto error;
> +	}
> +
>   	dp->catalog = dp_catalog_get(dev, &dp->parser->io);
>   	if (IS_ERR(dp->catalog)) {
>   		rc = PTR_ERR(dp->catalog);
> @@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
>   		goto error;
>   	}
>   
> +	rc = dp_power_client_init(dp->power);
> +	if (rc) {
> +		DRM_ERROR("Power client create failed\n");
> +		goto error;
> +	}
> +
>   	dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
>   	if (IS_ERR(dp->aux)) {
>   		rc = PTR_ERR(dp->aux);
> @@ -1196,26 +1202,20 @@ static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
>   	return ret;
>   }
>   
> -int dp_display_request_irq(struct msm_dp *dp_display)
> +static int dp_display_request_irq(struct dp_display_private *dp)
>   {
>   	int rc = 0;
> -	struct dp_display_private *dp;
> -
> -	if (!dp_display) {
> -		DRM_ERROR("invalid input\n");
> -		return -EINVAL;
> -	}
> -
> -	dp = container_of(dp_display, struct dp_display_private, dp_display);
> +	struct device *dev = &dp->pdev->dev;
>   
> -	dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>   	if (!dp->irq) {
> -		DRM_ERROR("failed to get irq\n");
> -		return -EINVAL;
> +		dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
> +		if (!dp->irq) {
> +			DRM_ERROR("failed to get irq\n");
> +			return -EINVAL;
> +		}
>   	}

Use platform_get_irq() from probe() function.

>   
> -	rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
> -			dp_display_irq_handler,
> +	rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
>   			IRQF_TRIGGER_HIGH, "dp_display_isr", dp);


>   	if (rc < 0) {
>   		DRM_ERROR("failed to request IRQ%u: %d\n",
> @@ -1290,6 +1290,8 @@ static int dp_display_probe(struct platform_device *pdev)
>   
>   	platform_set_drvdata(pdev, &dp->dp_display);
>   
> +	dp_display_request_irq(dp);
> +

Error checking?
Are we completely ready to handle interrupts at this point?

>   	rc = component_add(&pdev->dev, &dp_display_comp_ops);
>   	if (rc) {
>   		DRM_ERROR("component add failed, rc=%d\n", rc);
> @@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>   
>   	dp_priv = container_of(dp_display, struct dp_display_private, dp_display);
>   
> -	ret = dp_display_request_irq(dp_display);
> -	if (ret) {
> -		DRM_ERROR("request_irq failed, ret=%d\n", ret);
> -		return ret;
> -	}
> -
>   	ret = dp_display_get_next_bridge(dp_display);
>   	if (ret)
>   		return ret;
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
> index 1e9415a..b3c08de 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -35,7 +35,6 @@ struct msm_dp {
>   int dp_display_set_plugged_cb(struct msm_dp *dp_display,
>   		hdmi_codec_plugged_cb fn, struct device *codec_dev);
>   int dp_display_get_modes(struct msm_dp *dp_display);
> -int dp_display_request_irq(struct msm_dp *dp_display);
>   bool dp_display_check_video_test(struct msm_dp *dp_display);
>   int dp_display_get_test_bpp(struct msm_dp *dp_display);
>   void dp_display_signal_audio_start(struct msm_dp *dp_display);
Bjorn Andersson July 9, 2023, 3:09 a.m. UTC | #2
On Fri, Jul 07, 2023 at 04:52:22PM -0700, Kuogee Hsieh wrote:
> In preparation of moving edp of_dp_aux_populate_bus() to
> dp_display_probe(), move dp_display_request_irq(),
> dp->parser->parse() and dp_power_client_init() to dp_display_probe()
> too.
> 
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 48 +++++++++++++++++--------------------
>  drivers/gpu/drm/msm/dp/dp_display.h |  1 -
>  2 files changed, 22 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 44580c2..185f1eb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -290,12 +290,6 @@ static int dp_display_bind(struct device *dev, struct device *master,
>  		goto end;
>  	}
>  
> -	rc = dp_power_client_init(dp->power);
> -	if (rc) {
> -		DRM_ERROR("Power client create failed\n");
> -		goto end;
> -	}
> -
>  	rc = dp_register_audio_driver(dev, dp->audio);
>  	if (rc) {
>  		DRM_ERROR("Audio registration Dp failed\n");
> @@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
>  		goto error;
>  	}
>  
> +	rc = dp->parser->parse(dp->parser);

Today dp_init_sub_modules() just allocates memory for all the modules
and ties them together. While I don't fancy this way of structuring
device drivers in Linux, I think it's reasonable to retain that design
for now, and perform the parsing and power initialization in
dp_display_probe().

> +	if (rc) {
> +		DRM_ERROR("device tree parsing failed\n");
> +		goto error;
> +	}
> +
>  	dp->catalog = dp_catalog_get(dev, &dp->parser->io);
>  	if (IS_ERR(dp->catalog)) {
>  		rc = PTR_ERR(dp->catalog);
> @@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
>  		goto error;
>  	}
>  
> +	rc = dp_power_client_init(dp->power);
> +	if (rc) {
> +		DRM_ERROR("Power client create failed\n");
> +		goto error;
> +	}
> +
>  	dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
>  	if (IS_ERR(dp->aux)) {
>  		rc = PTR_ERR(dp->aux);
> @@ -1196,26 +1202,20 @@ static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
>  	return ret;
>  }
>  
> -int dp_display_request_irq(struct msm_dp *dp_display)
> +static int dp_display_request_irq(struct dp_display_private *dp)
>  {
>  	int rc = 0;
> -	struct dp_display_private *dp;
> -
> -	if (!dp_display) {
> -		DRM_ERROR("invalid input\n");
> -		return -EINVAL;
> -	}

Love this, but it's unrelated to the rest of the patch.

> -
> -	dp = container_of(dp_display, struct dp_display_private, dp_display);
> +	struct device *dev = &dp->pdev->dev;
>  
> -	dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>  	if (!dp->irq) {
> -		DRM_ERROR("failed to get irq\n");
> -		return -EINVAL;
> +		dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
> +		if (!dp->irq) {
> +			DRM_ERROR("failed to get irq\n");
> +			return -EINVAL;
> +		}
>  	}
>  
> -	rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
> -			dp_display_irq_handler,
> +	rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,

This is fixing a bug where currently the dp_display_irq_handler()
registration is tied to the DPU device's life cycle, while depending on
resources that are released as the DP device is torn down.

It would be nice if this was not hidden in a patch that claims to just
move calls around.

Regards,
Bjorn

>  			IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
>  	if (rc < 0) {
>  		DRM_ERROR("failed to request IRQ%u: %d\n",
> @@ -1290,6 +1290,8 @@ static int dp_display_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, &dp->dp_display);
>  
> +	dp_display_request_irq(dp);
> +
>  	rc = component_add(&pdev->dev, &dp_display_comp_ops);
>  	if (rc) {
>  		DRM_ERROR("component add failed, rc=%d\n", rc);
> @@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>  
>  	dp_priv = container_of(dp_display, struct dp_display_private, dp_display);
>  
> -	ret = dp_display_request_irq(dp_display);
> -	if (ret) {
> -		DRM_ERROR("request_irq failed, ret=%d\n", ret);
> -		return ret;
> -	}
> -
>  	ret = dp_display_get_next_bridge(dp_display);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
> index 1e9415a..b3c08de 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -35,7 +35,6 @@ struct msm_dp {
>  int dp_display_set_plugged_cb(struct msm_dp *dp_display,
>  		hdmi_codec_plugged_cb fn, struct device *codec_dev);
>  int dp_display_get_modes(struct msm_dp *dp_display);
> -int dp_display_request_irq(struct msm_dp *dp_display);
>  bool dp_display_check_video_test(struct msm_dp *dp_display);
>  int dp_display_get_test_bpp(struct msm_dp *dp_display);
>  void dp_display_signal_audio_start(struct msm_dp *dp_display);
> -- 
> 2.7.4
>
Kuogee Hsieh July 10, 2023, 4:57 p.m. UTC | #3
On 7/7/2023 5:11 PM, Dmitry Baryshkov wrote:
> On 08/07/2023 02:52, Kuogee Hsieh wrote:
>> In preparation of moving edp of_dp_aux_populate_bus() to
>> dp_display_probe(), move dp_display_request_irq(),
>> dp->parser->parse() and dp_power_client_init() to dp_display_probe()
>> too.
>>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_display.c | 48 
>> +++++++++++++++++--------------------
>>   drivers/gpu/drm/msm/dp/dp_display.h |  1 -
>>   2 files changed, 22 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>> b/drivers/gpu/drm/msm/dp/dp_display.c
>> index 44580c2..185f1eb 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -290,12 +290,6 @@ static int dp_display_bind(struct device *dev, 
>> struct device *master,
>>           goto end;
>>       }
>>   -    rc = dp_power_client_init(dp->power);
>> -    if (rc) {
>> -        DRM_ERROR("Power client create failed\n");
>> -        goto end;
>> -    }
>> -
>>       rc = dp_register_audio_driver(dev, dp->audio);
>>       if (rc) {
>>           DRM_ERROR("Audio registration Dp failed\n");
>> @@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct 
>> dp_display_private *dp)
>>           goto error;
>>       }
>>   +    rc = dp->parser->parse(dp->parser);
>> +    if (rc) {
>> +        DRM_ERROR("device tree parsing failed\n");
>> +        goto error;
>> +    }
>> +
>>       dp->catalog = dp_catalog_get(dev, &dp->parser->io);
>>       if (IS_ERR(dp->catalog)) {
>>           rc = PTR_ERR(dp->catalog);
>> @@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct 
>> dp_display_private *dp)
>>           goto error;
>>       }
>>   +    rc = dp_power_client_init(dp->power);
>> +    if (rc) {
>> +        DRM_ERROR("Power client create failed\n");
>> +        goto error;
>> +    }
>> +
>>       dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
>>       if (IS_ERR(dp->aux)) {
>>           rc = PTR_ERR(dp->aux);
>> @@ -1196,26 +1202,20 @@ static irqreturn_t dp_display_irq_handler(int 
>> irq, void *dev_id)
>>       return ret;
>>   }
>>   -int dp_display_request_irq(struct msm_dp *dp_display)
>> +static int dp_display_request_irq(struct dp_display_private *dp)
>>   {
>>       int rc = 0;
>> -    struct dp_display_private *dp;
>> -
>> -    if (!dp_display) {
>> -        DRM_ERROR("invalid input\n");
>> -        return -EINVAL;
>> -    }
>> -
>> -    dp = container_of(dp_display, struct dp_display_private, 
>> dp_display);
>> +    struct device *dev = &dp->pdev->dev;
>>   -    dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>>       if (!dp->irq) {
>> -        DRM_ERROR("failed to get irq\n");
>> -        return -EINVAL;
>> +        dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>> +        if (!dp->irq) {
>> +            DRM_ERROR("failed to get irq\n");
>> +            return -EINVAL;
>> +        }
>>       }
>
> Use platform_get_irq() from probe() function.
>
>>   -    rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
>> -            dp_display_irq_handler,
>> +    rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
>>               IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
>
>
>>       if (rc < 0) {
>>           DRM_ERROR("failed to request IRQ%u: %d\n",
>> @@ -1290,6 +1290,8 @@ static int dp_display_probe(struct 
>> platform_device *pdev)
>>         platform_set_drvdata(pdev, &dp->dp_display);
>>   +    dp_display_request_irq(dp);
>> +
>
> Error checking?
> Are we completely ready to handle interrupts at this point?
not until dp_display_host_init() is called which will be called from 
pm_runtime_resume() later.
>
>>       rc = component_add(&pdev->dev, &dp_display_comp_ops);
>>       if (rc) {
>>           DRM_ERROR("component add failed, rc=%d\n", rc);
>> @@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp 
>> *dp_display, struct drm_device *dev,
>>         dp_priv = container_of(dp_display, struct dp_display_private, 
>> dp_display);
>>   -    ret = dp_display_request_irq(dp_display);
>> -    if (ret) {
>> -        DRM_ERROR("request_irq failed, ret=%d\n", ret);
>> -        return ret;
>> -    }
>> -
>>       ret = dp_display_get_next_bridge(dp_display);
>>       if (ret)
>>           return ret;
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
>> b/drivers/gpu/drm/msm/dp/dp_display.h
>> index 1e9415a..b3c08de 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
>> @@ -35,7 +35,6 @@ struct msm_dp {
>>   int dp_display_set_plugged_cb(struct msm_dp *dp_display,
>>           hdmi_codec_plugged_cb fn, struct device *codec_dev);
>>   int dp_display_get_modes(struct msm_dp *dp_display);
>> -int dp_display_request_irq(struct msm_dp *dp_display);
>>   bool dp_display_check_video_test(struct msm_dp *dp_display);
>>   int dp_display_get_test_bpp(struct msm_dp *dp_display);
>>   void dp_display_signal_audio_start(struct msm_dp *dp_display);
>
Dmitry Baryshkov July 10, 2023, 6:13 p.m. UTC | #4
On 10/07/2023 19:57, Kuogee Hsieh wrote:
> 
> On 7/7/2023 5:11 PM, Dmitry Baryshkov wrote:
>> On 08/07/2023 02:52, Kuogee Hsieh wrote:
>>> In preparation of moving edp of_dp_aux_populate_bus() to
>>> dp_display_probe(), move dp_display_request_irq(),
>>> dp->parser->parse() and dp_power_client_init() to dp_display_probe()
>>> too.
>>>
>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>>> ---
>>>   drivers/gpu/drm/msm/dp/dp_display.c | 48 
>>> +++++++++++++++++--------------------
>>>   drivers/gpu/drm/msm/dp/dp_display.h |  1 -
>>>   2 files changed, 22 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>>> b/drivers/gpu/drm/msm/dp/dp_display.c
>>> index 44580c2..185f1eb 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>> @@ -290,12 +290,6 @@ static int dp_display_bind(struct device *dev, 
>>> struct device *master,
>>>           goto end;
>>>       }
>>>   -    rc = dp_power_client_init(dp->power);
>>> -    if (rc) {
>>> -        DRM_ERROR("Power client create failed\n");
>>> -        goto end;
>>> -    }
>>> -
>>>       rc = dp_register_audio_driver(dev, dp->audio);
>>>       if (rc) {
>>>           DRM_ERROR("Audio registration Dp failed\n");
>>> @@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct 
>>> dp_display_private *dp)
>>>           goto error;
>>>       }
>>>   +    rc = dp->parser->parse(dp->parser);
>>> +    if (rc) {
>>> +        DRM_ERROR("device tree parsing failed\n");
>>> +        goto error;
>>> +    }
>>> +
>>>       dp->catalog = dp_catalog_get(dev, &dp->parser->io);
>>>       if (IS_ERR(dp->catalog)) {
>>>           rc = PTR_ERR(dp->catalog);
>>> @@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct 
>>> dp_display_private *dp)
>>>           goto error;
>>>       }
>>>   +    rc = dp_power_client_init(dp->power);
>>> +    if (rc) {
>>> +        DRM_ERROR("Power client create failed\n");
>>> +        goto error;
>>> +    }
>>> +
>>>       dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
>>>       if (IS_ERR(dp->aux)) {
>>>           rc = PTR_ERR(dp->aux);
>>> @@ -1196,26 +1202,20 @@ static irqreturn_t dp_display_irq_handler(int 
>>> irq, void *dev_id)
>>>       return ret;
>>>   }
>>>   -int dp_display_request_irq(struct msm_dp *dp_display)
>>> +static int dp_display_request_irq(struct dp_display_private *dp)
>>>   {
>>>       int rc = 0;
>>> -    struct dp_display_private *dp;
>>> -
>>> -    if (!dp_display) {
>>> -        DRM_ERROR("invalid input\n");
>>> -        return -EINVAL;
>>> -    }
>>> -
>>> -    dp = container_of(dp_display, struct dp_display_private, 
>>> dp_display);
>>> +    struct device *dev = &dp->pdev->dev;
>>>   -    dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>>>       if (!dp->irq) {
>>> -        DRM_ERROR("failed to get irq\n");
>>> -        return -EINVAL;
>>> +        dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>>> +        if (!dp->irq) {
>>> +            DRM_ERROR("failed to get irq\n");
>>> +            return -EINVAL;
>>> +        }
>>>       }
>>
>> Use platform_get_irq() from probe() function.
>>
>>>   -    rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
>>> -            dp_display_irq_handler,
>>> +    rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
>>>               IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
>>
>>
>>>       if (rc < 0) {
>>>           DRM_ERROR("failed to request IRQ%u: %d\n",
>>> @@ -1290,6 +1290,8 @@ static int dp_display_probe(struct 
>>> platform_device *pdev)
>>>         platform_set_drvdata(pdev, &dp->dp_display);
>>>   +    dp_display_request_irq(dp);
>>> +
>>
>> Error checking?
>> Are we completely ready to handle interrupts at this point?
> not until dp_display_host_init() is called which will be called from 
> pm_runtime_resume() later.

But once you request_irq(), you should be ready for the IRQs to be 
delivered right away.

>>
>>>       rc = component_add(&pdev->dev, &dp_display_comp_ops);
>>>       if (rc) {
>>>           DRM_ERROR("component add failed, rc=%d\n", rc);
>>> @@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp 
>>> *dp_display, struct drm_device *dev,
>>>         dp_priv = container_of(dp_display, struct dp_display_private, 
>>> dp_display);
>>>   -    ret = dp_display_request_irq(dp_display);
>>> -    if (ret) {
>>> -        DRM_ERROR("request_irq failed, ret=%d\n", ret);
>>> -        return ret;
>>> -    }
>>> -
>>>       ret = dp_display_get_next_bridge(dp_display);
>>>       if (ret)
>>>           return ret;
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
>>> b/drivers/gpu/drm/msm/dp/dp_display.h
>>> index 1e9415a..b3c08de 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_display.h
>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
>>> @@ -35,7 +35,6 @@ struct msm_dp {
>>>   int dp_display_set_plugged_cb(struct msm_dp *dp_display,
>>>           hdmi_codec_plugged_cb fn, struct device *codec_dev);
>>>   int dp_display_get_modes(struct msm_dp *dp_display);
>>> -int dp_display_request_irq(struct msm_dp *dp_display);
>>>   bool dp_display_check_video_test(struct msm_dp *dp_display);
>>>   int dp_display_get_test_bpp(struct msm_dp *dp_display);
>>>   void dp_display_signal_audio_start(struct msm_dp *dp_display);
>>
Kuogee Hsieh July 17, 2023, 5:16 p.m. UTC | #5
On 7/10/2023 11:13 AM, Dmitry Baryshkov wrote:
> On 10/07/2023 19:57, Kuogee Hsieh wrote:
>>
>> On 7/7/2023 5:11 PM, Dmitry Baryshkov wrote:
>>> On 08/07/2023 02:52, Kuogee Hsieh wrote:
>>>> In preparation of moving edp of_dp_aux_populate_bus() to
>>>> dp_display_probe(), move dp_display_request_irq(),
>>>> dp->parser->parse() and dp_power_client_init() to dp_display_probe()
>>>> too.
>>>>
>>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>>>> ---
>>>>   drivers/gpu/drm/msm/dp/dp_display.c | 48 
>>>> +++++++++++++++++--------------------
>>>>   drivers/gpu/drm/msm/dp/dp_display.h |  1 -
>>>>   2 files changed, 22 insertions(+), 27 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>>>> b/drivers/gpu/drm/msm/dp/dp_display.c
>>>> index 44580c2..185f1eb 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>>> @@ -290,12 +290,6 @@ static int dp_display_bind(struct device *dev, 
>>>> struct device *master,
>>>>           goto end;
>>>>       }
>>>>   -    rc = dp_power_client_init(dp->power);
>>>> -    if (rc) {
>>>> -        DRM_ERROR("Power client create failed\n");
>>>> -        goto end;
>>>> -    }
>>>> -
>>>>       rc = dp_register_audio_driver(dev, dp->audio);
>>>>       if (rc) {
>>>>           DRM_ERROR("Audio registration Dp failed\n");
>>>> @@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct 
>>>> dp_display_private *dp)
>>>>           goto error;
>>>>       }
>>>>   +    rc = dp->parser->parse(dp->parser);
>>>> +    if (rc) {
>>>> +        DRM_ERROR("device tree parsing failed\n");
>>>> +        goto error;
>>>> +    }
>>>> +
>>>>       dp->catalog = dp_catalog_get(dev, &dp->parser->io);
>>>>       if (IS_ERR(dp->catalog)) {
>>>>           rc = PTR_ERR(dp->catalog);
>>>> @@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct 
>>>> dp_display_private *dp)
>>>>           goto error;
>>>>       }
>>>>   +    rc = dp_power_client_init(dp->power);
>>>> +    if (rc) {
>>>> +        DRM_ERROR("Power client create failed\n");
>>>> +        goto error;
>>>> +    }
>>>> +
>>>>       dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
>>>>       if (IS_ERR(dp->aux)) {
>>>>           rc = PTR_ERR(dp->aux);
>>>> @@ -1196,26 +1202,20 @@ static irqreturn_t 
>>>> dp_display_irq_handler(int irq, void *dev_id)
>>>>       return ret;
>>>>   }
>>>>   -int dp_display_request_irq(struct msm_dp *dp_display)
>>>> +static int dp_display_request_irq(struct dp_display_private *dp)
>>>>   {
>>>>       int rc = 0;
>>>> -    struct dp_display_private *dp;
>>>> -
>>>> -    if (!dp_display) {
>>>> -        DRM_ERROR("invalid input\n");
>>>> -        return -EINVAL;
>>>> -    }
>>>> -
>>>> -    dp = container_of(dp_display, struct dp_display_private, 
>>>> dp_display);
>>>> +    struct device *dev = &dp->pdev->dev;
>>>>   -    dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>>>>       if (!dp->irq) {
>>>> -        DRM_ERROR("failed to get irq\n");
>>>> -        return -EINVAL;
>>>> +        dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>>>> +        if (!dp->irq) {
>>>> +            DRM_ERROR("failed to get irq\n");
>>>> +            return -EINVAL;
>>>> +        }
>>>>       }
>>>
>>> Use platform_get_irq() from probe() function.
>>>
>>>>   -    rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
>>>> -            dp_display_irq_handler,
>>>> +    rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
>>>>               IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
>>>
>>>
>>>>       if (rc < 0) {
>>>>           DRM_ERROR("failed to request IRQ%u: %d\n",
>>>> @@ -1290,6 +1290,8 @@ static int dp_display_probe(struct 
>>>> platform_device *pdev)
>>>>         platform_set_drvdata(pdev, &dp->dp_display);
>>>>   +    dp_display_request_irq(dp);
>>>> +
>>>
>>> Error checking?
>>> Are we completely ready to handle interrupts at this point?
>> not until dp_display_host_init() is called which will be called from 
>> pm_runtime_resume() later.
>
> But once you request_irq(), you should be ready for the IRQs to be 
> delivered right away.

At this point, the DP controller interrupts mask bit is not enabled yet.

Therefore interrupts will not happen until dp_bridge_hpd_enable() is 
called to initialize dp host  controller and then enabled mask bits.

>
>>>
>>>>       rc = component_add(&pdev->dev, &dp_display_comp_ops);
>>>>       if (rc) {
>>>>           DRM_ERROR("component add failed, rc=%d\n", rc);
>>>> @@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp 
>>>> *dp_display, struct drm_device *dev,
>>>>         dp_priv = container_of(dp_display, struct 
>>>> dp_display_private, dp_display);
>>>>   -    ret = dp_display_request_irq(dp_display);
>>>> -    if (ret) {
>>>> -        DRM_ERROR("request_irq failed, ret=%d\n", ret);
>>>> -        return ret;
>>>> -    }
>>>> -
>>>>       ret = dp_display_get_next_bridge(dp_display);
>>>>       if (ret)
>>>>           return ret;
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
>>>> b/drivers/gpu/drm/msm/dp/dp_display.h
>>>> index 1e9415a..b3c08de 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.h
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
>>>> @@ -35,7 +35,6 @@ struct msm_dp {
>>>>   int dp_display_set_plugged_cb(struct msm_dp *dp_display,
>>>>           hdmi_codec_plugged_cb fn, struct device *codec_dev);
>>>>   int dp_display_get_modes(struct msm_dp *dp_display);
>>>> -int dp_display_request_irq(struct msm_dp *dp_display);
>>>>   bool dp_display_check_video_test(struct msm_dp *dp_display);
>>>>   int dp_display_get_test_bpp(struct msm_dp *dp_display);
>>>>   void dp_display_signal_audio_start(struct msm_dp *dp_display);
>>>
>
Dmitry Baryshkov July 17, 2023, 5:22 p.m. UTC | #6
On 17/07/2023 20:16, Kuogee Hsieh wrote:
> 
> On 7/10/2023 11:13 AM, Dmitry Baryshkov wrote:
>> On 10/07/2023 19:57, Kuogee Hsieh wrote:
>>>
>>> On 7/7/2023 5:11 PM, Dmitry Baryshkov wrote:
>>>> On 08/07/2023 02:52, Kuogee Hsieh wrote:
>>>>> In preparation of moving edp of_dp_aux_populate_bus() to
>>>>> dp_display_probe(), move dp_display_request_irq(),
>>>>> dp->parser->parse() and dp_power_client_init() to dp_display_probe()
>>>>> too.
>>>>>
>>>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>>>>> ---
>>>>>   drivers/gpu/drm/msm/dp/dp_display.c | 48 
>>>>> +++++++++++++++++--------------------
>>>>>   drivers/gpu/drm/msm/dp/dp_display.h |  1 -
>>>>>   2 files changed, 22 insertions(+), 27 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>>>>> b/drivers/gpu/drm/msm/dp/dp_display.c
>>>>> index 44580c2..185f1eb 100644
>>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>>>> @@ -290,12 +290,6 @@ static int dp_display_bind(struct device *dev, 
>>>>> struct device *master,
>>>>>           goto end;
>>>>>       }
>>>>>   -    rc = dp_power_client_init(dp->power);
>>>>> -    if (rc) {
>>>>> -        DRM_ERROR("Power client create failed\n");
>>>>> -        goto end;
>>>>> -    }
>>>>> -
>>>>>       rc = dp_register_audio_driver(dev, dp->audio);
>>>>>       if (rc) {
>>>>>           DRM_ERROR("Audio registration Dp failed\n");
>>>>> @@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct 
>>>>> dp_display_private *dp)
>>>>>           goto error;
>>>>>       }
>>>>>   +    rc = dp->parser->parse(dp->parser);
>>>>> +    if (rc) {
>>>>> +        DRM_ERROR("device tree parsing failed\n");
>>>>> +        goto error;
>>>>> +    }
>>>>> +
>>>>>       dp->catalog = dp_catalog_get(dev, &dp->parser->io);
>>>>>       if (IS_ERR(dp->catalog)) {
>>>>>           rc = PTR_ERR(dp->catalog);
>>>>> @@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct 
>>>>> dp_display_private *dp)
>>>>>           goto error;
>>>>>       }
>>>>>   +    rc = dp_power_client_init(dp->power);
>>>>> +    if (rc) {
>>>>> +        DRM_ERROR("Power client create failed\n");
>>>>> +        goto error;
>>>>> +    }
>>>>> +
>>>>>       dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
>>>>>       if (IS_ERR(dp->aux)) {
>>>>>           rc = PTR_ERR(dp->aux);
>>>>> @@ -1196,26 +1202,20 @@ static irqreturn_t 
>>>>> dp_display_irq_handler(int irq, void *dev_id)
>>>>>       return ret;
>>>>>   }
>>>>>   -int dp_display_request_irq(struct msm_dp *dp_display)
>>>>> +static int dp_display_request_irq(struct dp_display_private *dp)
>>>>>   {
>>>>>       int rc = 0;
>>>>> -    struct dp_display_private *dp;
>>>>> -
>>>>> -    if (!dp_display) {
>>>>> -        DRM_ERROR("invalid input\n");
>>>>> -        return -EINVAL;
>>>>> -    }
>>>>> -
>>>>> -    dp = container_of(dp_display, struct dp_display_private, 
>>>>> dp_display);
>>>>> +    struct device *dev = &dp->pdev->dev;
>>>>>   -    dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>>>>>       if (!dp->irq) {
>>>>> -        DRM_ERROR("failed to get irq\n");
>>>>> -        return -EINVAL;
>>>>> +        dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>>>>> +        if (!dp->irq) {
>>>>> +            DRM_ERROR("failed to get irq\n");
>>>>> +            return -EINVAL;
>>>>> +        }
>>>>>       }
>>>>
>>>> Use platform_get_irq() from probe() function.
>>>>
>>>>>   -    rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
>>>>> -            dp_display_irq_handler,
>>>>> +    rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
>>>>>               IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
>>>>
>>>>
>>>>>       if (rc < 0) {
>>>>>           DRM_ERROR("failed to request IRQ%u: %d\n",
>>>>> @@ -1290,6 +1290,8 @@ static int dp_display_probe(struct 
>>>>> platform_device *pdev)
>>>>>         platform_set_drvdata(pdev, &dp->dp_display);
>>>>>   +    dp_display_request_irq(dp);
>>>>> +
>>>>
>>>> Error checking?
>>>> Are we completely ready to handle interrupts at this point?
>>> not until dp_display_host_init() is called which will be called from 
>>> pm_runtime_resume() later.
>>
>> But once you request_irq(), you should be ready for the IRQs to be 
>> delivered right away.
> 
> At this point, the DP controller interrupts mask bit is not enabled yet.
> 
> Therefore interrupts will not happen until dp_bridge_hpd_enable() is 
> called to initialize dp host  controller and then enabled mask bits.

Are AUX and CTRL interrupts also disabled? What about any stray/pending 
interrupts? Just take it as a rule of thumb. Once request_irq() has been 
called without the IRQ_NOAUTOEN flag, the driver should be prepared to 
handle the incoming interrupt requests.

>>>>>       rc = component_add(&pdev->dev, &dp_display_comp_ops);
>>>>>       if (rc) {
>>>>>           DRM_ERROR("component add failed, rc=%d\n", rc);
>>>>> @@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp 
>>>>> *dp_display, struct drm_device *dev,
>>>>>         dp_priv = container_of(dp_display, struct 
>>>>> dp_display_private, dp_display);
>>>>>   -    ret = dp_display_request_irq(dp_display);
>>>>> -    if (ret) {
>>>>> -        DRM_ERROR("request_irq failed, ret=%d\n", ret);
>>>>> -        return ret;
>>>>> -    }
>>>>> -
>>>>>       ret = dp_display_get_next_bridge(dp_display);
>>>>>       if (ret)
>>>>>           return ret;
>>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
>>>>> b/drivers/gpu/drm/msm/dp/dp_display.h
>>>>> index 1e9415a..b3c08de 100644
>>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.h
>>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
>>>>> @@ -35,7 +35,6 @@ struct msm_dp {
>>>>>   int dp_display_set_plugged_cb(struct msm_dp *dp_display,
>>>>>           hdmi_codec_plugged_cb fn, struct device *codec_dev);
>>>>>   int dp_display_get_modes(struct msm_dp *dp_display);
>>>>> -int dp_display_request_irq(struct msm_dp *dp_display);
>>>>>   bool dp_display_check_video_test(struct msm_dp *dp_display);
>>>>>   int dp_display_get_test_bpp(struct msm_dp *dp_display);
>>>>>   void dp_display_signal_audio_start(struct msm_dp *dp_display);
>>>>
>>
Kuogee Hsieh July 17, 2023, 8:38 p.m. UTC | #7
On 7/17/2023 10:22 AM, Dmitry Baryshkov wrote:
> On 17/07/2023 20:16, Kuogee Hsieh wrote:
>>
>> On 7/10/2023 11:13 AM, Dmitry Baryshkov wrote:
>>> On 10/07/2023 19:57, Kuogee Hsieh wrote:
>>>>
>>>> On 7/7/2023 5:11 PM, Dmitry Baryshkov wrote:
>>>>> On 08/07/2023 02:52, Kuogee Hsieh wrote:
>>>>>> In preparation of moving edp of_dp_aux_populate_bus() to
>>>>>> dp_display_probe(), move dp_display_request_irq(),
>>>>>> dp->parser->parse() and dp_power_client_init() to dp_display_probe()
>>>>>> too.
>>>>>>
>>>>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/msm/dp/dp_display.c | 48 
>>>>>> +++++++++++++++++--------------------
>>>>>>   drivers/gpu/drm/msm/dp/dp_display.h |  1 -
>>>>>>   2 files changed, 22 insertions(+), 27 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>>>>>> b/drivers/gpu/drm/msm/dp/dp_display.c
>>>>>> index 44580c2..185f1eb 100644
>>>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>>>>> @@ -290,12 +290,6 @@ static int dp_display_bind(struct device 
>>>>>> *dev, struct device *master,
>>>>>>           goto end;
>>>>>>       }
>>>>>>   -    rc = dp_power_client_init(dp->power);
>>>>>> -    if (rc) {
>>>>>> -        DRM_ERROR("Power client create failed\n");
>>>>>> -        goto end;
>>>>>> -    }
>>>>>> -
>>>>>>       rc = dp_register_audio_driver(dev, dp->audio);
>>>>>>       if (rc) {
>>>>>>           DRM_ERROR("Audio registration Dp failed\n");
>>>>>> @@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct 
>>>>>> dp_display_private *dp)
>>>>>>           goto error;
>>>>>>       }
>>>>>>   +    rc = dp->parser->parse(dp->parser);
>>>>>> +    if (rc) {
>>>>>> +        DRM_ERROR("device tree parsing failed\n");
>>>>>> +        goto error;
>>>>>> +    }
>>>>>> +
>>>>>>       dp->catalog = dp_catalog_get(dev, &dp->parser->io);
>>>>>>       if (IS_ERR(dp->catalog)) {
>>>>>>           rc = PTR_ERR(dp->catalog);
>>>>>> @@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct 
>>>>>> dp_display_private *dp)
>>>>>>           goto error;
>>>>>>       }
>>>>>>   +    rc = dp_power_client_init(dp->power);
>>>>>> +    if (rc) {
>>>>>> +        DRM_ERROR("Power client create failed\n");
>>>>>> +        goto error;
>>>>>> +    }
>>>>>> +
>>>>>>       dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
>>>>>>       if (IS_ERR(dp->aux)) {
>>>>>>           rc = PTR_ERR(dp->aux);
>>>>>> @@ -1196,26 +1202,20 @@ static irqreturn_t 
>>>>>> dp_display_irq_handler(int irq, void *dev_id)
>>>>>>       return ret;
>>>>>>   }
>>>>>>   -int dp_display_request_irq(struct msm_dp *dp_display)
>>>>>> +static int dp_display_request_irq(struct dp_display_private *dp)
>>>>>>   {
>>>>>>       int rc = 0;
>>>>>> -    struct dp_display_private *dp;
>>>>>> -
>>>>>> -    if (!dp_display) {
>>>>>> -        DRM_ERROR("invalid input\n");
>>>>>> -        return -EINVAL;
>>>>>> -    }
>>>>>> -
>>>>>> -    dp = container_of(dp_display, struct dp_display_private, 
>>>>>> dp_display);
>>>>>> +    struct device *dev = &dp->pdev->dev;
>>>>>>   -    dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>>>>>>       if (!dp->irq) {
>>>>>> -        DRM_ERROR("failed to get irq\n");
>>>>>> -        return -EINVAL;
>>>>>> +        dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
>>>>>> +        if (!dp->irq) {
>>>>>> +            DRM_ERROR("failed to get irq\n");
>>>>>> +            return -EINVAL;
>>>>>> +        }
>>>>>>       }
>>>>>
>>>>> Use platform_get_irq() from probe() function.
>>>>>
>>>>>>   -    rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
>>>>>> -            dp_display_irq_handler,
>>>>>> +    rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
>>>>>>               IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
>>>>>
>>>>>
>>>>>>       if (rc < 0) {
>>>>>>           DRM_ERROR("failed to request IRQ%u: %d\n",
>>>>>> @@ -1290,6 +1290,8 @@ static int dp_display_probe(struct 
>>>>>> platform_device *pdev)
>>>>>>         platform_set_drvdata(pdev, &dp->dp_display);
>>>>>>   +    dp_display_request_irq(dp);
>>>>>> +
>>>>>
>>>>> Error checking?
>>>>> Are we completely ready to handle interrupts at this point?
>>>> not until dp_display_host_init() is called which will be called 
>>>> from pm_runtime_resume() later.
>>>
>>> But once you request_irq(), you should be ready for the IRQs to be 
>>> delivered right away.
>>
>> At this point, the DP controller interrupts mask bit is not enabled yet.
>>
>> Therefore interrupts will not happen until dp_bridge_hpd_enable() is 
>> called to initialize dp host controller and then enabled mask bits.
>
> Are AUX and CTRL interrupts also disabled? What about any 
> stray/pending interrupts? Just take it as a rule of thumb. Once 
> request_irq() has been called without the IRQ_NOAUTOEN flag, the 
> driver should be prepared to handle the incoming interrupt requests.

yes, both AUX and CTRL are disabled.

edp population do need irq to handle aux transfer during probe.

it should work by checking core_initialized flag at irq handle to filter 
out stray/pending interrupts.

>
>>>>>>       rc = component_add(&pdev->dev, &dp_display_comp_ops);
>>>>>>       if (rc) {
>>>>>>           DRM_ERROR("component add failed, rc=%d\n", rc);
>>>>>> @@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp 
>>>>>> *dp_display, struct drm_device *dev,
>>>>>>         dp_priv = container_of(dp_display, struct 
>>>>>> dp_display_private, dp_display);
>>>>>>   -    ret = dp_display_request_irq(dp_display);
>>>>>> -    if (ret) {
>>>>>> -        DRM_ERROR("request_irq failed, ret=%d\n", ret);
>>>>>> -        return ret;
>>>>>> -    }
>>>>>> -
>>>>>>       ret = dp_display_get_next_bridge(dp_display);
>>>>>>       if (ret)
>>>>>>           return ret;
>>>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
>>>>>> b/drivers/gpu/drm/msm/dp/dp_display.h
>>>>>> index 1e9415a..b3c08de 100644
>>>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.h
>>>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
>>>>>> @@ -35,7 +35,6 @@ struct msm_dp {
>>>>>>   int dp_display_set_plugged_cb(struct msm_dp *dp_display,
>>>>>>           hdmi_codec_plugged_cb fn, struct device *codec_dev);
>>>>>>   int dp_display_get_modes(struct msm_dp *dp_display);
>>>>>> -int dp_display_request_irq(struct msm_dp *dp_display);
>>>>>>   bool dp_display_check_video_test(struct msm_dp *dp_display);
>>>>>>   int dp_display_get_test_bpp(struct msm_dp *dp_display);
>>>>>>   void dp_display_signal_audio_start(struct msm_dp *dp_display);
>>>>>
>>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 44580c2..185f1eb 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -290,12 +290,6 @@  static int dp_display_bind(struct device *dev, struct device *master,
 		goto end;
 	}
 
-	rc = dp_power_client_init(dp->power);
-	if (rc) {
-		DRM_ERROR("Power client create failed\n");
-		goto end;
-	}
-
 	rc = dp_register_audio_driver(dev, dp->audio);
 	if (rc) {
 		DRM_ERROR("Audio registration Dp failed\n");
@@ -752,6 +746,12 @@  static int dp_init_sub_modules(struct dp_display_private *dp)
 		goto error;
 	}
 
+	rc = dp->parser->parse(dp->parser);
+	if (rc) {
+		DRM_ERROR("device tree parsing failed\n");
+		goto error;
+	}
+
 	dp->catalog = dp_catalog_get(dev, &dp->parser->io);
 	if (IS_ERR(dp->catalog)) {
 		rc = PTR_ERR(dp->catalog);
@@ -768,6 +768,12 @@  static int dp_init_sub_modules(struct dp_display_private *dp)
 		goto error;
 	}
 
+	rc = dp_power_client_init(dp->power);
+	if (rc) {
+		DRM_ERROR("Power client create failed\n");
+		goto error;
+	}
+
 	dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
 	if (IS_ERR(dp->aux)) {
 		rc = PTR_ERR(dp->aux);
@@ -1196,26 +1202,20 @@  static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
 	return ret;
 }
 
-int dp_display_request_irq(struct msm_dp *dp_display)
+static int dp_display_request_irq(struct dp_display_private *dp)
 {
 	int rc = 0;
-	struct dp_display_private *dp;
-
-	if (!dp_display) {
-		DRM_ERROR("invalid input\n");
-		return -EINVAL;
-	}
-
-	dp = container_of(dp_display, struct dp_display_private, dp_display);
+	struct device *dev = &dp->pdev->dev;
 
-	dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
 	if (!dp->irq) {
-		DRM_ERROR("failed to get irq\n");
-		return -EINVAL;
+		dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
+		if (!dp->irq) {
+			DRM_ERROR("failed to get irq\n");
+			return -EINVAL;
+		}
 	}
 
-	rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
-			dp_display_irq_handler,
+	rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
 			IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
 	if (rc < 0) {
 		DRM_ERROR("failed to request IRQ%u: %d\n",
@@ -1290,6 +1290,8 @@  static int dp_display_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, &dp->dp_display);
 
+	dp_display_request_irq(dp);
+
 	rc = component_add(&pdev->dev, &dp_display_comp_ops);
 	if (rc) {
 		DRM_ERROR("component add failed, rc=%d\n", rc);
@@ -1574,12 +1576,6 @@  int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 
 	dp_priv = container_of(dp_display, struct dp_display_private, dp_display);
 
-	ret = dp_display_request_irq(dp_display);
-	if (ret) {
-		DRM_ERROR("request_irq failed, ret=%d\n", ret);
-		return ret;
-	}
-
 	ret = dp_display_get_next_bridge(dp_display);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index 1e9415a..b3c08de 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -35,7 +35,6 @@  struct msm_dp {
 int dp_display_set_plugged_cb(struct msm_dp *dp_display,
 		hdmi_codec_plugged_cb fn, struct device *codec_dev);
 int dp_display_get_modes(struct msm_dp *dp_display);
-int dp_display_request_irq(struct msm_dp *dp_display);
 bool dp_display_check_video_test(struct msm_dp *dp_display);
 int dp_display_get_test_bpp(struct msm_dp *dp_display);
 void dp_display_signal_audio_start(struct msm_dp *dp_display);