diff mbox series

[v4,47/78] drm/vc4: hdmi: Retrieve the vc4_hdmi at unbind using our device

Message ID 0fc40024f73b3bc6d9eba0b47da217ed130c3374.1594230107.git-series.maxime@cerno.tech (mailing list archive)
State New, archived
Headers show
Series drm/vc4: Support BCM2711 Display Pipeline | expand

Commit Message

Maxime Ripard July 8, 2020, 5:41 p.m. UTC
The unbind function needs to retrieve a vc4_hdmi structure pointer through
the struct device that we're given since we want to support multiple HDMI
controllers.

However, our optional ASoC support doesn't make that trivial since it will
overwrite the device drvdata if we use it, but obviously won't if we don't
use it.

Let's make sure the fields are at the proper offset to be able to cast
between the snd_soc_card structure and the vc4_hdmi structure
transparently so we can support both cases.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 24 +++++++++++++++++++++++-
 drivers/gpu/drm/vc4/vc4_hdmi.h |  4 ++--
 2 files changed, 25 insertions(+), 3 deletions(-)

Comments

Dave Stevenson July 28, 2020, 12:32 p.m. UTC | #1
Hi Maxime

On Wed, 8 Jul 2020 at 18:43, Maxime Ripard <maxime@cerno.tech> wrote:
>
> The unbind function needs to retrieve a vc4_hdmi structure pointer through
> the struct device that we're given since we want to support multiple HDMI
> controllers.
>
> However, our optional ASoC support doesn't make that trivial since it will
> overwrite the device drvdata if we use it, but obviously won't if we don't
> use it.
>
> Let's make sure the fields are at the proper offset to be able to cast
> between the snd_soc_card structure and the vc4_hdmi structure
> transparently so we can support both cases.
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 24 +++++++++++++++++++++++-
>  drivers/gpu/drm/vc4/vc4_hdmi.h |  4 ++--
>  2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 09b297a1b39d..7cd1394c10fa 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1200,6 +1200,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
>         if (!vc4_hdmi)
>                 return -ENOMEM;
>
> +       dev_set_drvdata(dev, vc4_hdmi);
>         encoder = &vc4_hdmi->encoder.base.base;
>         vc4_hdmi->encoder.base.type = VC4_ENCODER_TYPE_HDMI0;
>         vc4_hdmi->pdev = pdev;
> @@ -1362,7 +1363,28 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master,
>  {
>         struct drm_device *drm = dev_get_drvdata(master);
>         struct vc4_dev *vc4 = drm->dev_private;
> -       struct vc4_hdmi *vc4_hdmi = vc4->hdmi;
> +       struct vc4_hdmi *vc4_hdmi;
> +
> +       /*
> +        * ASoC makes it a bit hard to retrieve a pointer to the
> +        * vc4_hdmi structure. Registering the card will overwrite our
> +        * device drvdata with a pointer to the snd_soc_card structure,
> +        * which can then be used to retrieve whatever drvdata we want
> +        * to associate.
> +        *
> +        * However, that doesn't fly in the case where we wouldn't
> +        * register an ASoC card (because of an old DT that is missing
> +        * the dmas properties for example), then the card isn't
> +        * registered and the device drvdata wouldn't be set.
> +        *
> +        * We can deal with both cases by making sure a snd_soc_card
> +        * pointer and a vc4_hdmi structure are pointing to the same
> +        * memory address, so we can treat them indistinctly without any
> +        * issue.
> +        */
> +       BUILD_BUG_ON(offsetof(struct vc4_hdmi_audio, card) != 0);
> +       BUILD_BUG_ON(offsetof(struct vc4_hdmi, audio) != 0);
> +       vc4_hdmi = dev_get_drvdata(dev);
>
>         cec_unregister_adapter(vc4_hdmi->cec_adap);
>         vc4_hdmi_connector_destroy(&vc4_hdmi->connector.base);
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> index 749a807cd1f3..d43462789450 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> @@ -53,13 +53,13 @@ struct vc4_hdmi_audio {
>
>  /* General HDMI hardware state. */
>  struct vc4_hdmi {
> +       struct vc4_hdmi_audio audio;
> +
>         struct platform_device *pdev;
>
>         struct vc4_hdmi_encoder encoder;
>         struct vc4_hdmi_connector connector;
>
> -       struct vc4_hdmi_audio audio;
> -
>         struct i2c_adapter *ddc;
>         void __iomem *hdmicore_regs;
>         void __iomem *hd_regs;
> --
> git-series 0.9.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 09b297a1b39d..7cd1394c10fa 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1200,6 +1200,7 @@  static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	if (!vc4_hdmi)
 		return -ENOMEM;
 
+	dev_set_drvdata(dev, vc4_hdmi);
 	encoder = &vc4_hdmi->encoder.base.base;
 	vc4_hdmi->encoder.base.type = VC4_ENCODER_TYPE_HDMI0;
 	vc4_hdmi->pdev = pdev;
@@ -1362,7 +1363,28 @@  static void vc4_hdmi_unbind(struct device *dev, struct device *master,
 {
 	struct drm_device *drm = dev_get_drvdata(master);
 	struct vc4_dev *vc4 = drm->dev_private;
-	struct vc4_hdmi *vc4_hdmi = vc4->hdmi;
+	struct vc4_hdmi *vc4_hdmi;
+
+	/*
+	 * ASoC makes it a bit hard to retrieve a pointer to the
+	 * vc4_hdmi structure. Registering the card will overwrite our
+	 * device drvdata with a pointer to the snd_soc_card structure,
+	 * which can then be used to retrieve whatever drvdata we want
+	 * to associate.
+	 *
+	 * However, that doesn't fly in the case where we wouldn't
+	 * register an ASoC card (because of an old DT that is missing
+	 * the dmas properties for example), then the card isn't
+	 * registered and the device drvdata wouldn't be set.
+	 *
+	 * We can deal with both cases by making sure a snd_soc_card
+	 * pointer and a vc4_hdmi structure are pointing to the same
+	 * memory address, so we can treat them indistinctly without any
+	 * issue.
+	 */
+	BUILD_BUG_ON(offsetof(struct vc4_hdmi_audio, card) != 0);
+	BUILD_BUG_ON(offsetof(struct vc4_hdmi, audio) != 0);
+	vc4_hdmi = dev_get_drvdata(dev);
 
 	cec_unregister_adapter(vc4_hdmi->cec_adap);
 	vc4_hdmi_connector_destroy(&vc4_hdmi->connector.base);
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 749a807cd1f3..d43462789450 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -53,13 +53,13 @@  struct vc4_hdmi_audio {
 
 /* General HDMI hardware state. */
 struct vc4_hdmi {
+	struct vc4_hdmi_audio audio;
+
 	struct platform_device *pdev;
 
 	struct vc4_hdmi_encoder encoder;
 	struct vc4_hdmi_connector connector;
 
-	struct vc4_hdmi_audio audio;
-
 	struct i2c_adapter *ddc;
 	void __iomem *hdmicore_regs;
 	void __iomem *hd_regs;