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

[Robert Foss <robert.foss@linaro.org>]

Hey Nikolaus,

Thanks for submitting this series.

On Thu, 5 Aug 2021 at 16:08, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> From: Paul Boddie <paul@boddie.org.uk>
>
> 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@boddie.org.uk>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  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@ravnborg.org/

>         .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
>