Re: [PATCH] drm/bridge: tc358768: Enforce input bus flags via atomic_check

[Leonardo Costa <leoreis.costa@gmail.com>]

Hello,

> The tc358768 declares static bridge timings requiring pixel data to be
> sampled on the positive clock edge.
> 
> However, the DRM core default propagation simply copies the output-side
> bus flags, coming from the next bridge, connector or panel, to the
> input side. If the propagated flags are incompatible with the bridge
> ones, the data is wrongly sampled, typically resulting in visual
> artifacts on the panel.
> 
> Implement the atomic_check hook, replacing the mutually exclusive
> mode_fixup, and set the bridge state input bus flags to the ones
> required by the tc358768. The sync polarity defaulting previously done
> in mode_fixup is carried over into atomic_check unchanged.
> 
> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Leonardo Costa <leonardo.costa@toradex.com>
> ---
>  drivers/gpu/drm/bridge/tc358768.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> index 0d85120fcc7a3..0516a331e71ba 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -1262,10 +1262,13 @@ tc358768_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>  	return input_fmts;
>  }
>  
> -static bool tc358768_mode_fixup(struct drm_bridge *bridge,
> -				const struct drm_display_mode *mode,
> -				struct drm_display_mode *adjusted_mode)
> +static int tc358768_bridge_atomic_check(struct drm_bridge *bridge,
> +					struct drm_bridge_state *bridge_state,
> +					struct drm_crtc_state *crtc_state,
> +					struct drm_connector_state *conn_state)
>  {
> +	struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
> +
>  	/* Default to positive sync */
>  
>  	if (!(adjusted_mode->flags &
> @@ -1276,13 +1279,15 @@ static bool tc358768_mode_fixup(struct drm_bridge *bridge,
>  	      (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
>  		adjusted_mode->flags |= DRM_MODE_FLAG_PVSYNC;
>  
> -	return true;
> +	bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags;
> +
> +	return 0;
>  }
>  
>  static const struct drm_bridge_funcs tc358768_bridge_funcs = {
>  	.attach = tc358768_bridge_attach,
>  	.mode_valid = tc358768_bridge_mode_valid,
> -	.mode_fixup = tc358768_mode_fixup,
> +	.atomic_check = tc358768_bridge_atomic_check,
>  	.atomic_pre_enable = tc358768_bridge_atomic_pre_enable,
>  	.atomic_enable = tc358768_bridge_atomic_enable,
>  	.atomic_disable = tc358768_bridge_atomic_disable,

Please ignore this, as it was not meant to be sent in reply to the thread
above. That was an accident.