Hi,

On 06/04/2022 18:26, H. Nikolaus Schaller wrote:
From: Neil Armstrong <narmstr...@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 <narmstr...@baylibre.com>
Signed-off-by: H. Nikolaus Schaller <h...@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

Reply via email to