Re: [PATCH v2 1/8] drm/bridge: synopsis: Add mode_fixup and bridge timings support

From: Robert Foss
Date: Thu Aug 05 2021 - 10:32:35 EST


Hey Nikolaus,

Thanks for submitting this series.

On Thu, 5 Aug 2021 at 16:08, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> wrote:
>
> From: Paul Boddie <paul@xxxxxxxxxxxxx>
>
> The platform-specific configuration structure is augmented with
> mode_fixup and timings members so that specialisations of the
> Synopsys driver can introduce mode flags and bus flags.
>
> Signed-off-by: Paul Boddie <paul@xxxxxxxxxxxxx>
> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> Signed-off-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 16 ++++++++++++++++
> include/drm/bridge/dw_hdmi.h | 5 +++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index e7c7c9b9c646f..e8499eb11328c 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2810,6 +2810,19 @@ dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
> return mode_status;
> }
>
> +static bool
> +dw_hdmi_bridge_mode_fixup(struct drm_bridge *bridge,
> + const struct drm_display_mode *mode,
> + struct drm_display_mode *adjusted_mode)
> +{
> + struct dw_hdmi *hdmi = bridge->driver_private;
> +
> + if (hdmi->plat_data->mode_fixup)
> + return hdmi->plat_data->mode_fixup(bridge, mode, adjusted_mode);
> +
> + return true;
> +}
> +
> static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> const struct drm_display_mode *orig_mode,
> const struct drm_display_mode *mode)
> @@ -2883,6 +2896,7 @@ static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
> .atomic_disable = dw_hdmi_bridge_atomic_disable,
> .mode_set = dw_hdmi_bridge_mode_set,
> .mode_valid = dw_hdmi_bridge_mode_valid,
> + .mode_fixup = dw_hdmi_bridge_mode_fixup,

mode_fixup() has been deprecated[1] in favor of atomic_check(), care
has to be taken when switching to atomic_check() as it has access to
the full atomic commit.

Looking at this driver, it's using mode_set as well, which should be fixed.

[1] https://lore.kernel.org/dri-devel/20210722062246.2512666-8-sam@xxxxxxxxxxxx/

> .detect = dw_hdmi_bridge_detect,
> .get_edid = dw_hdmi_bridge_get_edid,
> };
> @@ -3364,6 +3378,8 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> #ifdef CONFIG_OF
> hdmi->bridge.of_node = pdev->dev.of_node;
> #endif
> + if (plat_data->timings)
> + hdmi->bridge.timings = plat_data->timings;
>
> memset(&pdevinfo, 0, sizeof(pdevinfo));
> pdevinfo.parent = dev;
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 6a5716655619b..677137445d534 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -8,6 +8,7 @@
>
> #include <sound/hdmi-codec.h>
>
> +struct drm_bridge;
> struct drm_display_info;
> struct drm_display_mode;
> struct drm_encoder;
> @@ -140,6 +141,10 @@ struct dw_hdmi_plat_data {
> enum drm_mode_status (*mode_valid)(struct dw_hdmi *hdmi, void *data,
> const struct drm_display_info *info,
> const struct drm_display_mode *mode);
> + bool (*mode_fixup)(struct drm_bridge *bridge,
> + const struct drm_display_mode *mode,
> + struct drm_display_mode *adjusted_mode);
> + const struct drm_bridge_timings *timings;
>
> /* Vendor PHY support */
> const struct dw_hdmi_phy_ops *phy_ops;
> --
> 2.31.1
>