[PATCH v17 4/6] drm/bridge: dw-hdmi: handle unusable or non-configured CSC module
Neil Armstrong
narmstrong at baylibre.com
Thu Apr 7 08:28:05 UTC 2022
Hi,
On 06/04/2022 18:26, H. Nikolaus Schaller wrote:
> From: Neil Armstrong <narmstrong at baylibre.com>
>
> The dw-hdmi integrates an optional Color Space Conversion feature used
> to handle color-space conversions.
>
> On some platforms, the CSC isn't built-in or non-functional.
>
> This adds the necessary code to disable the CSC functionality
> and limit the bus format negotiation to force using the same
> input bus format as the output bus format.
>
> Signed-off-by: Neil Armstrong <narmstrong at baylibre.com>
> Signed-off-by: H. Nikolaus Schaller <hns at goldelico.com>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 100 +++++++++++++++-------
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 1 +
> include/drm/bridge/dw_hdmi.h | 1 +
> 3 files changed, 71 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index f50af40e10340..b5a665c5e406e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -158,6 +158,8 @@ struct dw_hdmi {
> struct hdmi_data_info hdmi_data;
> const struct dw_hdmi_plat_data *plat_data;
>
> + bool csc_available; /* indicates if the CSC engine is usable */
> +
> int vic;
>
> u8 edid[HDMI_EDID_LEN];
> @@ -1009,9 +1011,10 @@ static int is_color_space_interpolation(struct dw_hdmi *hdmi)
>
> static bool is_csc_needed(struct dw_hdmi *hdmi)
> {
> - return is_color_space_conversion(hdmi) ||
> - is_color_space_decimation(hdmi) ||
> - is_color_space_interpolation(hdmi);
> + return hdmi->csc_available &&
> + (is_color_space_conversion(hdmi) ||
> + is_color_space_decimation(hdmi) ||
> + is_color_space_interpolation(hdmi));
> }
>
> static void dw_hdmi_update_csc_coeffs(struct dw_hdmi *hdmi)
> @@ -1064,6 +1067,9 @@ static void hdmi_video_csc(struct dw_hdmi *hdmi)
> int interpolation = HDMI_CSC_CFG_INTMODE_DISABLE;
> int decimation = 0;
>
> + if (!hdmi->csc_available)
> + return;
> +
> /* YCC422 interpolation to 444 mode */
> if (is_color_space_interpolation(hdmi))
> interpolation = HDMI_CSC_CFG_INTMODE_CHROMA_INT_FORMULA1;
> @@ -2665,6 +2671,7 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> u32 output_fmt,
> unsigned int *num_input_fmts)
> {
> + struct dw_hdmi *hdmi = bridge->driver_private;
> u32 *input_fmts;
> unsigned int i = 0;
>
> @@ -2683,62 +2690,81 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> /* 8bit */
> case MEDIA_BUS_FMT_RGB888_1X24:
> input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> + }
> break;
> case MEDIA_BUS_FMT_YUV8_1X24:
> input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> + }
> break;
> case MEDIA_BUS_FMT_UYVY8_1X16:
> input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> + }
> break;
>
> /* 10bit */
> case MEDIA_BUS_FMT_RGB101010_1X30:
> input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> + }
> break;
> case MEDIA_BUS_FMT_YUV10_1X30:
> input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> + }
> break;
> case MEDIA_BUS_FMT_UYVY10_1X20:
> input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> + }
> break;
>
> /* 12bit */
> case MEDIA_BUS_FMT_RGB121212_1X36:
> input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> + }
> break;
> case MEDIA_BUS_FMT_YUV12_1X36:
> input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> - input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> + }
> break;
> case MEDIA_BUS_FMT_UYVY12_1X24:
> input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> + if (hdmi->csc_available) {
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> + input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> + }
> break;
>
> /* 16bit */
> case MEDIA_BUS_FMT_RGB161616_1X48:
> input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> + if (hdmi->csc_available)
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> break;
> case MEDIA_BUS_FMT_YUV16_1X48:
> - input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> - input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
> + if (hdmi->csc_available)
> + input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> break;
>
> /*YUV 4:2:0 */
> @@ -2767,15 +2793,24 @@ static int dw_hdmi_bridge_atomic_check(struct drm_bridge *bridge,
> {
> struct dw_hdmi *hdmi = bridge->driver_private;
>
> - hdmi->hdmi_data.enc_out_bus_format =
> - bridge_state->output_bus_cfg.format;
> + if (!hdmi->csc_available &&
> + bridge_state->output_bus_cfg.format != bridge_state->input_bus_cfg.format) {
> + dev_warn(hdmi->dev, "different input format 0x%04x & output format 0x%04x while CSC isn't usable, fallback to safe format\n",
> + bridge_state->input_bus_cfg.format,
> + bridge_state->output_bus_cfg.format);
> + hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_FIXED;
> + hdmi->hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_FIXED;
> + } else {
> + hdmi->hdmi_data.enc_out_bus_format =
> + bridge_state->output_bus_cfg.format;
>
> - hdmi->hdmi_data.enc_in_bus_format =
> - bridge_state->input_bus_cfg.format;
> + hdmi->hdmi_data.enc_in_bus_format =
> + bridge_state->input_bus_cfg.format;
>
> - dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
> - bridge_state->input_bus_cfg.format,
> - bridge_state->output_bus_cfg.format);
> + dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
> + bridge_state->input_bus_cfg.format,
> + bridge_state->output_bus_cfg.format);
> + }
>
> return 0;
> }
> @@ -3481,6 +3516,9 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> hdmi->cec = platform_device_register_full(&pdevinfo);
> }
>
> + /* Get CSC useability from config0 register and permit override for platforms */
> + hdmi->csc_available = !plat_data->disable_csc || (config0 & HDMI_CONFIG0_CSC);
> +
> drm_bridge_add(&hdmi->bridge);
>
> return hdmi;
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> index 1999db05bc3b2..279722e4d1898 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> @@ -541,6 +541,7 @@ enum {
>
> /* CONFIG0_ID field values */
> HDMI_CONFIG0_I2S = 0x10,
> + HDMI_CONFIG0_CSC = 0x04,
> HDMI_CONFIG0_CEC = 0x02,
>
> /* CONFIG1_ID field values */
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 2a1f85f9a8a3f..b2f689cbe864c 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -157,6 +157,7 @@ struct dw_hdmi_plat_data {
> unsigned long mpixelclock);
>
> unsigned int disable_cec : 1;
> + unsigned int disable_csc : 1;
> };
>
> struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
Is this really still needed now you filter correctly the possible
modes in patch 1 ?
Neil
More information about the dri-devel
mailing list