On Fri, 10 Jun 2022 at 10:30, Maxime Ripard <maxime@xxxxxxxxxx> wrote:
>
> The VC4 DPI driver private structure contains only a pointer to the
> encoder it implements. This makes the overall structure somewhat
> inconsistent with the rest of the driver, and complicates its
> initialisation without any apparent gain.
>
> Let's embed the drm_encoder structure (through the vc4_encoder one) into
> struct vc4_dpi to fix both issues.
>
> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/vc4/vc4_dpi.c | 49 ++++++++++++-----------------------
> 1 file changed, 16 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index f2b46c524919..c88e8e397730 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -83,10 +83,10 @@
>
> /* General DPI hardware state. */
> struct vc4_dpi {
> + struct vc4_encoder encoder;
> +
> struct platform_device *pdev;
>
> - struct drm_encoder *encoder;
> -
> void __iomem *regs;
>
> struct clk *pixel_clock;
> @@ -95,21 +95,15 @@ struct vc4_dpi {
> struct debugfs_regset32 regset;
> };
>
> +static inline struct vc4_dpi *
> +to_vc4_dpi(struct drm_encoder *encoder)
> +{
> + return container_of(encoder, struct vc4_dpi, encoder.base);
> +}
> +
> #define DPI_READ(offset) readl(dpi->regs + (offset))
> #define DPI_WRITE(offset, val) writel(val, dpi->regs + (offset))
>
> -/* VC4 DPI encoder KMS struct */
> -struct vc4_dpi_encoder {
> - struct vc4_encoder base;
> - struct vc4_dpi *dpi;
> -};
> -
> -static inline struct vc4_dpi_encoder *
> -to_vc4_dpi_encoder(struct drm_encoder *encoder)
> -{
> - return container_of(encoder, struct vc4_dpi_encoder, base.base);
> -}
> -
> static const struct debugfs_reg32 dpi_regs[] = {
> VC4_REG32(DPI_C),
> VC4_REG32(DPI_ID),
> @@ -117,8 +111,7 @@ static const struct debugfs_reg32 dpi_regs[] = {
>
> static void vc4_dpi_encoder_disable(struct drm_encoder *encoder)
> {
> - struct vc4_dpi_encoder *vc4_encoder = to_vc4_dpi_encoder(encoder);
> - struct vc4_dpi *dpi = vc4_encoder->dpi;
> + struct vc4_dpi *dpi = to_vc4_dpi(encoder);
>
> clk_disable_unprepare(dpi->pixel_clock);
> }
> @@ -127,8 +120,7 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder)
> {
> struct drm_device *dev = encoder->dev;
> struct drm_display_mode *mode = &encoder->crtc->mode;
> - struct vc4_dpi_encoder *vc4_encoder = to_vc4_dpi_encoder(encoder);
> - struct vc4_dpi *dpi = vc4_encoder->dpi;
> + struct vc4_dpi *dpi = to_vc4_dpi(encoder);
> struct drm_connector_list_iter conn_iter;
> struct drm_connector *connector = NULL, *connector_scan;
> u32 dpi_c = DPI_ENABLE | DPI_OUTPUT_ENABLE_MODE;
> @@ -242,7 +234,7 @@ static int vc4_dpi_init_bridge(struct vc4_dpi *dpi)
> return PTR_ERR(bridge);
> }
>
> - return drm_bridge_attach(dpi->encoder, bridge, NULL, 0);
> + return drm_bridge_attach(&dpi->encoder.base, bridge, NULL, 0);
> }
>
> static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
> @@ -250,21 +242,12 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
> struct platform_device *pdev = to_platform_device(dev);
> struct drm_device *drm = dev_get_drvdata(master);
> struct vc4_dpi *dpi;
> - struct vc4_dpi_encoder *vc4_dpi_encoder;
> int ret;
>
> dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL);
> if (!dpi)
> return -ENOMEM;
> -
> - vc4_dpi_encoder = devm_kzalloc(dev, sizeof(*vc4_dpi_encoder),
> - GFP_KERNEL);
> - if (!vc4_dpi_encoder)
> - return -ENOMEM;
> - vc4_dpi_encoder->base.type = VC4_ENCODER_TYPE_DPI;
> - vc4_dpi_encoder->dpi = dpi;
> - dpi->encoder = &vc4_dpi_encoder->base.base;
> -
> + dpi->encoder.type = VC4_ENCODER_TYPE_DPI;
> dpi->pdev = pdev;
> dpi->regs = vc4_ioremap_regs(pdev, 0);
> if (IS_ERR(dpi->regs))
> @@ -298,8 +281,8 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
> if (ret)
> DRM_ERROR("Failed to turn on core clock: %d\n", ret);
>
> - drm_simple_encoder_init(drm, dpi->encoder, DRM_MODE_ENCODER_DPI);
> - drm_encoder_helper_add(dpi->encoder, &vc4_dpi_encoder_helper_funcs);
> + drm_simple_encoder_init(drm, &dpi->encoder.base, DRM_MODE_ENCODER_DPI);
> + drm_encoder_helper_add(&dpi->encoder.base, &vc4_dpi_encoder_helper_funcs);
>
> ret = vc4_dpi_init_bridge(dpi);
> if (ret)
> @@ -312,7 +295,7 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
> return 0;
>
> err_destroy_encoder:
> - drm_encoder_cleanup(dpi->encoder);
> + drm_encoder_cleanup(&dpi->encoder.base);
> clk_disable_unprepare(dpi->core_clock);
> return ret;
> }
> @@ -324,7 +307,7 @@ static void vc4_dpi_unbind(struct device *dev, struct device *master,
>
> drm_of_panel_bridge_remove(dev->of_node, 0, 0);
>
> - drm_encoder_cleanup(dpi->encoder);
> + drm_encoder_cleanup(&dpi->encoder.base);
>
> clk_disable_unprepare(dpi->core_clock);
> }
> --
> 2.36.1
>
