Re: [PATCH RFC 2/5] drm/bridge: add encoder support to specify bridge input format

[Neil Armstrong]

Hi Laurent,

On 29/03/2019 11:42, Neil Armstrong wrote:
> This patch adds a new format_set() callback to the bridge ops permitting
> the encoder to specify the new input format and encoding.

Is this something better ? This doesn't violate the layering anymore,
do you have any thoughts about this new API ?

Neil

> 
> This allows supporting the very specific HDMI2.0 YUV420 output mode
> when the bridge cannot convert from RGB or YUV444 to YUV420.
> 
> In this case, the encode must downsample before the bridge and must
> specify the bridge the new input bus format differs.
> 
> This will also help supporting the YUV420 mode where the bridge cannot
> downsample, and also support 10bit, 12bit and 16bit output modes
> when the bridge cannot convert between different bit depths.
> 
> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_bridge.c | 35 +++++++++++++++++++++++++++++++++++
>  include/drm/drm_bridge.h     | 19 +++++++++++++++++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 138b2711d389..b2ce2d3d070e 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -307,6 +307,41 @@ void drm_bridge_mode_set(struct drm_bridge *bridge,
>  }
>  EXPORT_SYMBOL(drm_bridge_mode_set);
>  
> +/**
> + * drm_bridge_format_set - setup with proposed input format and encoding for
> + *			   all bridges in the encoder chain
> + * @bridge: bridge control structure
> + * @input_bus_format: proposed input bus format for the bridge
> + * @input_encoding: proposed input encoding for this bridge
> + *
> + * Calls &drm_bridge_funcs.format_set op for all the bridges in the
> + * encoder chain, starting from the first bridge to the last.
> + *
> + * Note: the bridge passed should be the one closest to the encoder
> + *
> + * RETURNS:
> + * true on success, false if one of the bridge cannot handle the format
> + */
> +bool drm_bridge_format_set(struct drm_bridge *bridge,
> +			   const u32 input_bus_format,
> +			   const u32 input_encoding)
> +{
> +	bool ret = true;
> +
> +	if (!bridge)
> +		return true;
> +
> +	if (bridge->funcs->format_set)
> +		ret = bridge->funcs->format_set(bridge, input_bus_format,
> +						input_encoding);
> +	if (ret)
> +		return ret;
> +
> +	return drm_bridge_format_set(bridge->next, input_bus_format,
> +				     input_encoding);
> +}
> +EXPORT_SYMBOL(drm_bridge_format_set);
> +
>  /**
>   * drm_bridge_pre_enable - prepares for enabling all
>   *			   bridges in the encoder chain
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 9da8c93f7976..223253b15763 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -198,6 +198,22 @@ struct drm_bridge_funcs {
>  	void (*mode_set)(struct drm_bridge *bridge,
>  			 const struct drm_display_mode *mode,
>  			 const struct drm_display_mode *adjusted_mode);
> +
> +	/**
> +	 * @format_set:
> +	 *
> +	 * This callback should configure the bridge for the given input bus
> +	 * format and encoding. It is called after the @format_set callback
> +	 * for the preceding element in the display pipeline has been called
> +	 * already. If the bridge is the first element then this would be
> +	 * &drm_encoder_helper_funcs.format_set. The display pipe (i.e.
> +	 * clocks and timing signals) is off when this function is called.
> +	 *
> +	 * @returns: true in success, false is a bridge refuses the format
> +	 */
> +	bool (*format_set)(struct drm_bridge *bridge,
> +			   const u32 input_bus_format,
> +			   const u32 input_encoding);
>  	/**
>  	 * @pre_enable:
>  	 *
> @@ -312,6 +328,9 @@ void drm_bridge_post_disable(struct drm_bridge *bridge);
>  void drm_bridge_mode_set(struct drm_bridge *bridge,
>  			 const struct drm_display_mode *mode,
>  			 const struct drm_display_mode *adjusted_mode);
> +bool drm_bridge_format_set(struct drm_bridge *bridge,
> +			   const u32 input_bus_format,
> +			   const u32 input_encoding);
>  void drm_bridge_pre_enable(struct drm_bridge *bridge);
>  void drm_bridge_enable(struct drm_bridge *bridge);
>  
>