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 <[email protected]>
---
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
Hi Maxime
On Wed, 8 Jul 2020 at 18:43, Maxime Ripard <[email protected]> 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 <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
> ---
> 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