devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
To: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
	wens-jdAy2FN1RRM@public.gmane.org,
	airlied-cv59FeDIM0c@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org,
	sboyd-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org,
	Laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org,
	narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org,
	Jose.Abreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	hjc-TNX95d0MmH7DzftRWevZcw@public.gmane.org,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
	p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH v5 05/12] drm/bridge/synopsys: dw-hdmi: don't clobber drvdata
Date: Fri, 16 Feb 2018 10:31:53 +0530	[thread overview]
Message-ID: <d96f32aa-cc37-44da-6b32-fde4db8ffe11@codeaurora.org> (raw)
In-Reply-To: <20180214200906.31509-6-jernej.skrabec-gGgVlfcn5nU@public.gmane.org>



On Thursday 15 February 2018 01:38 AM, Jernej Skrabec wrote:
> dw_hdmi shouldn't set drvdata since some drivers might need to store
> it's own data there. Rework dw_hdmi in a way to return struct dw_hdmi
> instead to store it in drvdata. This way drivers are responsible to
> store and pass structure when needed.
> 
> Idea was taken from the following commit:
> 8242ecbd597d ("drm/bridge/synopsys: stop clobbering drvdata")

Reviewed-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

> 
> Cc: p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
> Cc: narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org
> Cc: Laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org
> Cc: hjc-TNX95d0MmH7DzftRWevZcw@public.gmane.org
> Cc: heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org
> Acked-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c   | 31 ++++++++++++-----------------
>   drivers/gpu/drm/imx/dw_hdmi-imx.c           | 13 +++++++++---
>   drivers/gpu/drm/meson/meson_dw_hdmi.c       | 14 +++++++++----
>   drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c      | 12 +++++++++--
>   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 13 +++++++++---
>   include/drm/bridge/dw_hdmi.h                | 13 ++++++------
>   6 files changed, 60 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 7d80f4b56683..f9802399cc0d 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2543,8 +2543,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
>   	if (hdmi->i2c)
>   		dw_hdmi_i2c_init(hdmi);
>   
> -	platform_set_drvdata(pdev, hdmi);
> -
>   	return hdmi;
>   
>   err_iahb:
> @@ -2594,25 +2592,23 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
>   /* -----------------------------------------------------------------------------
>    * Probe/remove API, used from platforms based on the DRM bridge API.
>    */
> -int dw_hdmi_probe(struct platform_device *pdev,
> -		  const struct dw_hdmi_plat_data *plat_data)
> +struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> +			      const struct dw_hdmi_plat_data *plat_data)
>   {
>   	struct dw_hdmi *hdmi;
>   
>   	hdmi = __dw_hdmi_probe(pdev, plat_data);
>   	if (IS_ERR(hdmi))
> -		return PTR_ERR(hdmi);
> +		return hdmi;
>   
>   	drm_bridge_add(&hdmi->bridge);
>   
> -	return 0;
> +	return hdmi;
>   }
>   EXPORT_SYMBOL_GPL(dw_hdmi_probe);
>   
> -void dw_hdmi_remove(struct platform_device *pdev)
> +void dw_hdmi_remove(struct dw_hdmi *hdmi)
>   {
> -	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> -
>   	drm_bridge_remove(&hdmi->bridge);
>   
>   	__dw_hdmi_remove(hdmi);
> @@ -2622,31 +2618,30 @@ EXPORT_SYMBOL_GPL(dw_hdmi_remove);
>   /* -----------------------------------------------------------------------------
>    * Bind/unbind API, used from platforms based on the component framework.
>    */
> -int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
> -		 const struct dw_hdmi_plat_data *plat_data)
> +struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
> +			     struct drm_encoder *encoder,
> +			     const struct dw_hdmi_plat_data *plat_data)
>   {
>   	struct dw_hdmi *hdmi;
>   	int ret;
>   
>   	hdmi = __dw_hdmi_probe(pdev, plat_data);
>   	if (IS_ERR(hdmi))
> -		return PTR_ERR(hdmi);
> +		return hdmi;
>   
>   	ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL);
>   	if (ret) {
> -		dw_hdmi_remove(pdev);
> +		dw_hdmi_remove(hdmi);
>   		DRM_ERROR("Failed to initialize bridge with drm\n");
> -		return ret;
> +		return ERR_PTR(ret);
>   	}
>   
> -	return 0;
> +	return hdmi;
>   }
>   EXPORT_SYMBOL_GPL(dw_hdmi_bind);
>   
> -void dw_hdmi_unbind(struct device *dev)
> +void dw_hdmi_unbind(struct dw_hdmi *hdmi)
>   {
> -	struct dw_hdmi *hdmi = dev_get_drvdata(dev);
> -
>   	__dw_hdmi_remove(hdmi);
>   }
>   EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
> diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> index b62763aa8706..fe6becdcc29e 100644
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -25,6 +25,7 @@
>   struct imx_hdmi {
>   	struct device *dev;
>   	struct drm_encoder encoder;
> +	struct dw_hdmi *hdmi;
>   	struct regmap *regmap;
>   };
>   
> @@ -239,14 +240,18 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
>   	drm_encoder_init(drm, encoder, &dw_hdmi_imx_encoder_funcs,
>   			 DRM_MODE_ENCODER_TMDS, NULL);
>   
> -	ret = dw_hdmi_bind(pdev, encoder, plat_data);
> +	platform_set_drvdata(pdev, hdmi);
> +
> +	hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
>   
>   	/*
>   	 * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
>   	 * which would have called the encoder cleanup.  Do it manually.
>   	 */
> -	if (ret)
> +	if (IS_ERR(hdmi->hdmi)) {
> +		ret = PTR_ERR(hdmi->hdmi);
>   		drm_encoder_cleanup(encoder);
> +	}
>   
>   	return ret;
>   }
> @@ -254,7 +259,9 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
>   static void dw_hdmi_imx_unbind(struct device *dev, struct device *master,
>   			       void *data)
>   {
> -	return dw_hdmi_unbind(dev);
> +	struct imx_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	dw_hdmi_unbind(hdmi->hdmi);
>   }
>   
>   static const struct component_ops dw_hdmi_imx_ops = {
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index e8c3ef8a94ce..d49af17310c9 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -140,6 +140,7 @@ struct meson_dw_hdmi {
>   	struct clk *venci_clk;
>   	struct regulator *hdmi_supply;
>   	u32 irq_stat;
> +	struct dw_hdmi *hdmi;
>   };
>   #define encoder_to_meson_dw_hdmi(x) \
>   	container_of(x, struct meson_dw_hdmi, encoder)
> @@ -878,9 +879,12 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>   	dw_plat_data->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
>   	dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709;
>   
> -	ret = dw_hdmi_bind(pdev, encoder, &meson_dw_hdmi->dw_plat_data);
> -	if (ret)
> -		return ret;
> +	platform_set_drvdata(pdev, meson_dw_hdmi);
> +
> +	meson_dw_hdmi->hdmi = dw_hdmi_bind(pdev, encoder,
> +					   &meson_dw_hdmi->dw_plat_data);
> +	if (IS_ERR(meson_dw_hdmi->hdmi))
> +		return PTR_ERR(meson_dw_hdmi->hdmi);
>   
>   	DRM_DEBUG_DRIVER("HDMI controller initialized\n");
>   
> @@ -890,7 +894,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>   static void meson_dw_hdmi_unbind(struct device *dev, struct device *master,
>   				   void *data)
>   {
> -	dw_hdmi_unbind(dev);
> +	struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev);
> +
> +	dw_hdmi_unbind(meson_dw_hdmi->hdmi);
>   }
>   
>   static const struct component_ops meson_dw_hdmi_ops = {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> index dc85b53d58ef..3bebc6821e9c 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> @@ -68,12 +68,20 @@ static const struct dw_hdmi_plat_data rcar_dw_hdmi_plat_data = {
>   
>   static int rcar_dw_hdmi_probe(struct platform_device *pdev)
>   {
> -	return dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
> +	struct dw_hdmi *hdmi;
> +
> +	hdmi = dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
> +	if (IS_ERR(hdmi))
> +		return PTR_ERR(hdmi);
> +
> +	platform_set_drvdata(pdev, hdmi);
>   }
>   
>   static int rcar_dw_hdmi_remove(struct platform_device *pdev)
>   {
> -	dw_hdmi_remove(pdev);
> +	struct dw_hdmi *hdmi = platform_get_drvdata(dev);
> +
> +	dw_hdmi_remove(hdmi);
>   
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 1eb02a82fd91..3574b0ae2ad1 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -48,6 +48,7 @@ struct rockchip_hdmi {
>   	const struct rockchip_hdmi_chip_data *chip_data;
>   	struct clk *vpll_clk;
>   	struct clk *grf_clk;
> +	struct dw_hdmi *hdmi;
>   };
>   
>   #define to_rockchip_hdmi(x)	container_of(x, struct rockchip_hdmi, x)
> @@ -377,14 +378,18 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
>   	drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
>   			 DRM_MODE_ENCODER_TMDS, NULL);
>   
> -	ret = dw_hdmi_bind(pdev, encoder, plat_data);
> +	platform_set_drvdata(pdev, hdmi);
> +
> +	hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
>   
>   	/*
>   	 * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
>   	 * which would have called the encoder cleanup.  Do it manually.
>   	 */
> -	if (ret)
> +	if (IS_ERR(hdmi->hdmi)) {
> +		ret = PTR_ERR(hdmi->hdmi);
>   		drm_encoder_cleanup(encoder);
> +	}
>   
>   	return ret;
>   }
> @@ -392,7 +397,9 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
>   static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
>   				    void *data)
>   {
> -	return dw_hdmi_unbind(dev);
> +	struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	dw_hdmi_unbind(hdmi->hdmi);
>   }
>   
>   static const struct component_ops dw_hdmi_rockchip_ops = {
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index f3f3f0e1b2d3..dd2a8cf7d20b 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -143,12 +143,13 @@ struct dw_hdmi_plat_data {
>   			     unsigned long mpixelclock);
>   };
>   
> -int dw_hdmi_probe(struct platform_device *pdev,
> -		  const struct dw_hdmi_plat_data *plat_data);
> -void dw_hdmi_remove(struct platform_device *pdev);
> -void dw_hdmi_unbind(struct device *dev);
> -int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
> -		 const struct dw_hdmi_plat_data *plat_data);
> +struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> +			      const struct dw_hdmi_plat_data *plat_data);
> +void dw_hdmi_remove(struct dw_hdmi *hdmi);
> +void dw_hdmi_unbind(struct dw_hdmi *hdmi);
> +struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
> +			     struct drm_encoder *encoder,
> +			     const struct dw_hdmi_plat_data *plat_data);
>   
>   void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense);
>   
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2018-02-16  5:01 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-14 20:08 [PATCH v5 00/12] drm/sun4i: Add A83T HDMI support Jernej Skrabec
     [not found] ` <20180214200906.31509-1-jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
2018-02-14 20:08   ` [PATCH v5 01/12] clk: sunxi-ng: Mask nkmp factors when setting register Jernej Skrabec
2018-02-14 20:08   ` [PATCH v5 02/12] clk: sunxi-ng: Use u64 for calculation of nkmp rate Jernej Skrabec
2018-02-14 20:08   ` [PATCH v5 03/12] drm/bridge/synopsys: dw-hdmi: Enable workaround for v1.32a Jernej Skrabec
2018-02-16  5:01     ` Archit Taneja
2018-02-14 20:08   ` [PATCH v5 04/12] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions Jernej Skrabec
2018-02-16  5:01     ` Archit Taneja
2018-02-14 20:08   ` [PATCH v5 05/12] drm/bridge/synopsys: dw-hdmi: don't clobber drvdata Jernej Skrabec
     [not found]     ` <20180214200906.31509-6-jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
2018-02-16  5:01       ` Archit Taneja [this message]
2018-02-16  7:03     ` Heiko Stuebner
2018-02-19 13:49     ` Philipp Zabel
2018-02-14 20:09   ` [PATCH v5 06/12] dt-bindings: display: sun4i-drm: Add A83T HDMI pipeline Jernej Skrabec
2018-02-14 20:09   ` [PATCH v5 07/12] drm/sun4i: Add has_channel_0 TCON quirk Jernej Skrabec
2018-02-14 20:09   ` [PATCH v5 08/12] drm/sun4i: Add support for A83T second TCON Jernej Skrabec
2018-02-14 20:09   ` [PATCH v5 09/12] drm/sun4i: Add support for A83T second DE2 mixer Jernej Skrabec
2018-02-14 20:09   ` [PATCH v5 10/12] drm/sun4i: Implement A83T HDMI driver Jernej Skrabec
2018-02-14 21:42     ` Philippe Ombredanne
2018-02-14 20:09 ` [PATCH v5 11/12] ARM: dts: sun8i: a83t: Add HDMI display pipeline Jernej Skrabec
2018-02-14 20:09 ` [PATCH v5 12/12] ARM: dts: sun8i: a83t: Enable HDMI on BananaPi M3 Jernej Skrabec
2018-02-15 13:52 ` [PATCH v5 00/12] drm/sun4i: Add A83T HDMI support Maxime Ripard
2018-02-16  9:09 ` Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d96f32aa-cc37-44da-6b32-fde4db8ffe11@codeaurora.org \
    --to=architt-sgv2jx0feol9jmxxk+q4oq@public.gmane.org \
    --cc=Jose.Abreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org \
    --cc=Laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org \
    --cc=a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=airlied-cv59FeDIM0c@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org \
    --cc=hjc-TNX95d0MmH7DzftRWevZcw@public.gmane.org \
    --cc=jernej.skrabec-gGgVlfcn5nU@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
    --cc=mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org \
    --cc=narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org \
    --cc=p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sboyd-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=wens-jdAy2FN1RRM@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).