linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dpu: Remove dpu_mdss_isr when dpu_mdss_destroy is called
@ 2018-08-28 21:23 Jordan Crouse
       [not found] ` <20180828212304.13193-1-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
  0 siblings, 1 reply; 2+ messages in thread
From: Jordan Crouse @ 2018-08-28 21:23 UTC (permalink / raw)
  To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	jsanka-sgV2jX0FEOL9JmXXK+q4OQ, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	robdclark-Re5JQEeQqe8AvxtiuMwx3w,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

The MDSS device is created before the MSM driver attempts to bind the
sub components. If any of the components return -EPROBE_DEFER the MDSS
device is destroyed and tried again later.

If this happens the "dpu_mdss_isr" interrupt created from the DPU MDSS
is not freed when the MDSS device is destroyed and has a risk of
triggering later and hitting a fault by accessing a mmio region that
no longer exists. Even if the interrupt isn't triggered by
accident when the device attempts to reprobe it would error out
when it tries to re-register the interrupt so unconditionally removing
it in the destroy is the right move.

Switch the device managed "dpu_mdss_isr" to be unmanaged and add a
free_irq() in the mdss destroy function.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index 9e533b86682c..2235ef8129f4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -158,6 +158,8 @@ static void dpu_mdss_destroy(struct drm_device *dev)
 
 	_dpu_mdss_irq_domain_fini(dpu_mdss);
 
+	free_irq(platform_get_irq(pdev, 0), dpu_mdss);
+
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
 	devm_kfree(&pdev->dev, mp->clk_config);
 
@@ -215,7 +217,7 @@ int dpu_mdss_init(struct drm_device *dev)
 	if (ret)
 		goto irq_domain_error;
 
-	ret = devm_request_irq(dev->dev, platform_get_irq(pdev, 0),
+	ret = request_irq(platform_get_irq(pdev, 0),
 			dpu_mdss_irq, 0, "dpu_mdss_isr", dpu_mdss);
 	if (ret) {
 		DPU_ERROR("failed to init irq: %d\n", ret);
-- 
2.18.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/msm/dpu: Remove dpu_mdss_isr when dpu_mdss_destroy is called
       [not found] ` <20180828212304.13193-1-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
@ 2018-08-31 17:53   ` Sean Paul
  0 siblings, 0 replies; 2+ messages in thread
From: Sean Paul @ 2018-08-31 17:53 UTC (permalink / raw)
  To: Jordan Crouse
  Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	robdclark-Re5JQEeQqe8AvxtiuMwx3w, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Tue, Aug 28, 2018 at 03:23:04PM -0600, Jordan Crouse wrote:
> The MDSS device is created before the MSM driver attempts to bind the
> sub components. If any of the components return -EPROBE_DEFER the MDSS
> device is destroyed and tried again later.
> 
> If this happens the "dpu_mdss_isr" interrupt created from the DPU MDSS
> is not freed when the MDSS device is destroyed and has a risk of
> triggering later and hitting a fault by accessing a mmio region that
> no longer exists. Even if the interrupt isn't triggered by
> accident when the device attempts to reprobe it would error out
> when it tries to re-register the interrupt so unconditionally removing
> it in the destroy is the right move.
> 
> Switch the device managed "dpu_mdss_isr" to be unmanaged and add a
> free_irq() in the mdss destroy function.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> index 9e533b86682c..2235ef8129f4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> @@ -158,6 +158,8 @@ static void dpu_mdss_destroy(struct drm_device *dev)
>  
>  	_dpu_mdss_irq_domain_fini(dpu_mdss);
>  
> +	free_irq(platform_get_irq(pdev, 0), dpu_mdss);
> +
>  	msm_dss_put_clk(mp->clk_config, mp->num_clk);
>  	devm_kfree(&pdev->dev, mp->clk_config);
>  
> @@ -215,7 +217,7 @@ int dpu_mdss_init(struct drm_device *dev)
>  	if (ret)
>  		goto irq_domain_error;
>  
> -	ret = devm_request_irq(dev->dev, platform_get_irq(pdev, 0),
> +	ret = request_irq(platform_get_irq(pdev, 0),
>  			dpu_mdss_irq, 0, "dpu_mdss_isr", dpu_mdss);
>  	if (ret) {
>  		DPU_ERROR("failed to init irq: %d\n", ret);
> -- 
> 2.18.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-08-31 17:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-28 21:23 [PATCH] drm/msm/dpu: Remove dpu_mdss_isr when dpu_mdss_destroy is called Jordan Crouse
     [not found] ` <20180828212304.13193-1-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-08-31 17:53   ` Sean Paul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).