diff mbox series

[v2,2/4] drm/omap: hdmi4: Ensure the device is active during bind

Message ID 20181105151028.4091-3-laurent.pinchart@ideasonboard.com
State New, archived
Headers show
Series omapdrm: Fix runtime PM issues at module load and unload time | expand

Commit Message

Laurent Pinchart Nov. 5, 2018, 3:10 p.m. UTC
The bind function performs hardware access (in hdmi4_cec_init()) and
thus requires the device to be active. Ensure this by surrounding the
bind function by hdmi_runtime_get() and hdmi_runtime_put() calls.

Fixes: 27d624527d99 ("drm/omap: dss: Acquire next dssdev at probe time")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/dss/hdmi4.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Tony Lindgren Nov. 5, 2018, 8:02 p.m. UTC | #1
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [181105 15:10]:
> The bind function performs hardware access (in hdmi4_cec_init()) and
> thus requires the device to be active. Ensure this by surrounding the
> bind function by hdmi_runtime_get() and hdmi_runtime_put() calls.
> 
> Fixes: 27d624527d99 ("drm/omap: dss: Acquire next dssdev at probe time")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Looks good to me except one typo below..

> index cf6230eac31a..36be9a36d664 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> @@ -635,10 +635,14 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>  
>  	hdmi->dss = dss;
>  
> -	r = hdmi_pll_init(dss, hdmi->pdev, &hdmi->pll, &hdmi->wp);
> +	r = hdmi_runtime_get(hdmi);
>  	if (r)
>  		return r;
>  
> +	r = hdmi_pll_init(dss, hdmi->pdev, &hdmi->pll, &hdmi->wp);
> +	if (r)
> +		goto err_runtime_put;
> +
>  	r = hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp);
>  	if (r)
>  		goto err_pll_uninit;
> @@ -652,12 +656,16 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>  	hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs,
>  					       hdmi);
>  
> +	hdmi_runtime_put(hdmi);
> +
>  	return 0;
>  
>  err_cec_uninit:
>  	hdmi4_cec_uninit(&hdmi->core);
>  err_pll_uninit:
>  	hdmi_pll_uninit(&hdmi->pll);
> +err_runtime_put:
> +	hdmi_runtime_get(hdmi);
>  	return r;
>  }

At err_runtime_put you should call hdmi_runtime_put() instead of
hdmi_runtime_get(), right? :)

Regards,

Tony
Laurent Pinchart Nov. 5, 2018, 9:45 p.m. UTC | #2
Hi Tony,

On Monday, 5 November 2018 22:02:47 EET Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [181105 15:10]:
> > The bind function performs hardware access (in hdmi4_cec_init()) and
> > thus requires the device to be active. Ensure this by surrounding the
> > bind function by hdmi_runtime_get() and hdmi_runtime_put() calls.
> > 
> > Fixes: 27d624527d99 ("drm/omap: dss: Acquire next dssdev at probe time")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Looks good to me except one typo below..
> 
> > index cf6230eac31a..36be9a36d664 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c

[snip]

> > @@ -652,12 +656,16 @@ static int hdmi4_bind(struct device *dev, struct
> > device *master, void *data)
> >  	hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs,
> >  					       hdmi);
> > 
> > +	hdmi_runtime_put(hdmi);
> > +
> >  	return 0;
> >  
> >  err_cec_uninit:
> >  	hdmi4_cec_uninit(&hdmi->core);
> >  err_pll_uninit:
> >  	hdmi_pll_uninit(&hdmi->pll);
> > +err_runtime_put:
> > +	hdmi_runtime_get(hdmi);
> >  	return r;
> >  }
> 
> At err_runtime_put you should call hdmi_runtime_put() instead of
> hdmi_runtime_get(), right? :)

Oops. Of course. Thank you for the review. This will be fixed in v3.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index cf6230eac31a..36be9a36d664 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -635,10 +635,14 @@  static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 
 	hdmi->dss = dss;
 
-	r = hdmi_pll_init(dss, hdmi->pdev, &hdmi->pll, &hdmi->wp);
+	r = hdmi_runtime_get(hdmi);
 	if (r)
 		return r;
 
+	r = hdmi_pll_init(dss, hdmi->pdev, &hdmi->pll, &hdmi->wp);
+	if (r)
+		goto err_runtime_put;
+
 	r = hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp);
 	if (r)
 		goto err_pll_uninit;
@@ -652,12 +656,16 @@  static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 	hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs,
 					       hdmi);
 
+	hdmi_runtime_put(hdmi);
+
 	return 0;
 
 err_cec_uninit:
 	hdmi4_cec_uninit(&hdmi->core);
 err_pll_uninit:
 	hdmi_pll_uninit(&hdmi->pll);
+err_runtime_put:
+	hdmi_runtime_get(hdmi);
 	return r;
 }