diff mbox series

[RFC,v3,22/37] drm/rockchip: inno_hdmi: Remove useless output format

Message ID 20231031-kms-hdmi-connector-state-v3-22-328b0fae43a7@kernel.org (mailing list archive)
State New, archived
Headers show
Series drm/connector: Create HDMI Connector infrastructure | expand

Commit Message

Maxime Ripard Oct. 31, 2023, 4:48 p.m. UTC 
Similarly to the input format, the driver has a lot of code to deal with
various output format, but the driver hardcodes it to RGB always.

Let's get rid of the dead code.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/rockchip/inno_hdmi.c | 57 ++++--------------------------------
 1 file changed, 6 insertions(+), 51 deletions(-)

Comments

Johan Jonker Nov. 25, 2023, 10 a.m. UTC | #1 
Hi Maxime,

In stead of further cripplingRockchip HDMI drivers one could also make it functional based  on EDID info.

To start with the output could you turn RGB888 input and switch between

RGB444, YCBCR444 and YCBCR422 output.

Johan

On 10/31/23 17:48, Maxime Ripard wrote:
> Similarly to the input format, the driver has a lot of code to deal with
> various output format, but the driver hardcodes it to RGB always.
>
> Let's get rid of the dead code.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/rockchip/inno_hdmi.c | 57 ++++--------------------------------
>  1 file changed, 6 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index e0696ab16da3..0c6c550e0ce7 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -28,7 +28,6 @@
>  #include "inno_hdmi.h"
>  
>  struct hdmi_data_info {
> -	unsigned int enc_out_format;
>  	unsigned int colorimetry;
>  };
>  
> @@ -296,26 +295,14 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
>  	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
>  						      &hdmi->connector,
>  						      mode);
> -
> -	if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
> -		frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> -	else if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV422)
> -		frame.avi.colorspace = HDMI_COLORSPACE_YUV422;
> -	else
> -		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> +	frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>  
>  	return inno_hdmi_upload_frame(hdmi, rc, &frame, INFOFRAME_AVI, 0, 0, 0);
>  }
>  
>  static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
>  {
> -	struct hdmi_data_info *data = &hdmi->hdmi_data;
> -	int c0_c2_change = 0;
> -	int csc_enable = 0;
> -	int csc_mode = 0;
> -	int auto_csc = 0;
>  	int value;
> -	int i;
>  
>  	/* Input video mode is SDR RGB24bit, data enable signal from external */
>  	hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL1, v_DE_EXTERNAL |
> @@ -327,43 +314,13 @@ static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
>  		v_VIDEO_INPUT_CSP(0);
>  	hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL2, value);
>  
> -	if (HDMI_COLORSPACE_RGB == data->enc_out_format) {
> -		value = v_SOF_DISABLE | v_COLOR_DEPTH_NOT_INDICATED(1);
> -		hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL3, value);
> -
> -		hdmi_modb(hdmi, HDMI_VIDEO_CONTRL,
> -			  m_VIDEO_AUTO_CSC | m_VIDEO_C0_C2_SWAP,
> -			  v_VIDEO_AUTO_CSC(AUTO_CSC_DISABLE) |
> -			  v_VIDEO_C0_C2_SWAP(C0_C2_CHANGE_DISABLE));
> -		return 0;
> -	}
> -
> -	if (data->colorimetry == HDMI_COLORIMETRY_ITU_601) {
> -		if (data->enc_out_format == HDMI_COLORSPACE_YUV444) {
> -			csc_mode = CSC_RGB_0_255_TO_ITU601_16_235_8BIT;
> -			auto_csc = AUTO_CSC_DISABLE;
> -			c0_c2_change = C0_C2_CHANGE_DISABLE;
> -			csc_enable = v_CSC_ENABLE;
> -		}
> -	} else {
> -		if (data->enc_out_format == HDMI_COLORSPACE_YUV444) {
> -			csc_mode = CSC_RGB_0_255_TO_ITU709_16_235_8BIT;
> -			auto_csc = AUTO_CSC_DISABLE;
> -			c0_c2_change = C0_C2_CHANGE_DISABLE;
> -			csc_enable = v_CSC_ENABLE;
> -		}
> -	}
> -
> -	for (i = 0; i < 24; i++)
> -		hdmi_writeb(hdmi, HDMI_VIDEO_CSC_COEF + i,
> -			    coeff_csc[csc_mode][i]);
> -
> -	value = v_SOF_DISABLE | csc_enable | v_COLOR_DEPTH_NOT_INDICATED(1);
> +	value = v_SOF_DISABLE | v_COLOR_DEPTH_NOT_INDICATED(1);
>  	hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL3, value);
> -	hdmi_modb(hdmi, HDMI_VIDEO_CONTRL, m_VIDEO_AUTO_CSC |
> -		  m_VIDEO_C0_C2_SWAP, v_VIDEO_AUTO_CSC(auto_csc) |
> -		  v_VIDEO_C0_C2_SWAP(c0_c2_change));
>  
> +	hdmi_modb(hdmi, HDMI_VIDEO_CONTRL,
> +		  m_VIDEO_AUTO_CSC | m_VIDEO_C0_C2_SWAP,
> +		  v_VIDEO_AUTO_CSC(AUTO_CSC_DISABLE) |
> +		  v_VIDEO_C0_C2_SWAP(C0_C2_CHANGE_DISABLE));
>  	return 0;
>  }
>  
> @@ -425,8 +382,6 @@ static int inno_hdmi_setup(struct inno_hdmi *hdmi,
>  	struct drm_display_info *display = &hdmi->connector.display_info;
>  	u8 vic = drm_match_cea_mode(mode);
>  
> -	hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
> -
>  	if ((vic == 6) || (vic == 7) ||
>  	    (vic == 21) || (vic == 22) ||
>  	    (vic == 2) || (vic == 3) ||
>
Maxime Ripard Nov. 27, 2023, 9:40 a.m. UTC | #2 
Hi,

On Sat, Nov 25, 2023 at 11:00:52AM +0100, Johan Jonker wrote:
> In stead of further cripplingRockchip HDMI drivers one could also make
> it functional based  on EDID info.

I'm not crippling it, it was dead code. This has no functional impact
whatsoever.

> To start with the output could you turn RGB888 input and switch between
> RGB444, YCBCR444 and YCBCR422 output.

I don't have any hardware to test that out

Maxime
diff mbox series

Patch

diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index e0696ab16da3..0c6c550e0ce7 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -28,7 +28,6 @@ 
 #include "inno_hdmi.h"
 
 struct hdmi_data_info {
-	unsigned int enc_out_format;
 	unsigned int colorimetry;
 };
 
@@ -296,26 +295,14 @@  static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
 	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
 						      &hdmi->connector,
 						      mode);
-
-	if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
-		frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
-	else if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV422)
-		frame.avi.colorspace = HDMI_COLORSPACE_YUV422;
-	else
-		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
+	frame.avi.colorspace = HDMI_COLORSPACE_RGB;
 
 	return inno_hdmi_upload_frame(hdmi, rc, &frame, INFOFRAME_AVI, 0, 0, 0);
 }
 
 static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
 {
-	struct hdmi_data_info *data = &hdmi->hdmi_data;
-	int c0_c2_change = 0;
-	int csc_enable = 0;
-	int csc_mode = 0;
-	int auto_csc = 0;
 	int value;
-	int i;
 
 	/* Input video mode is SDR RGB24bit, data enable signal from external */
 	hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL1, v_DE_EXTERNAL |
@@ -327,43 +314,13 @@  static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
 		v_VIDEO_INPUT_CSP(0);
 	hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL2, value);
 
-	if (HDMI_COLORSPACE_RGB == data->enc_out_format) {
-		value = v_SOF_DISABLE | v_COLOR_DEPTH_NOT_INDICATED(1);
-		hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL3, value);
-
-		hdmi_modb(hdmi, HDMI_VIDEO_CONTRL,
-			  m_VIDEO_AUTO_CSC | m_VIDEO_C0_C2_SWAP,
-			  v_VIDEO_AUTO_CSC(AUTO_CSC_DISABLE) |
-			  v_VIDEO_C0_C2_SWAP(C0_C2_CHANGE_DISABLE));
-		return 0;
-	}
-
-	if (data->colorimetry == HDMI_COLORIMETRY_ITU_601) {
-		if (data->enc_out_format == HDMI_COLORSPACE_YUV444) {
-			csc_mode = CSC_RGB_0_255_TO_ITU601_16_235_8BIT;
-			auto_csc = AUTO_CSC_DISABLE;
-			c0_c2_change = C0_C2_CHANGE_DISABLE;
-			csc_enable = v_CSC_ENABLE;
-		}
-	} else {
-		if (data->enc_out_format == HDMI_COLORSPACE_YUV444) {
-			csc_mode = CSC_RGB_0_255_TO_ITU709_16_235_8BIT;
-			auto_csc = AUTO_CSC_DISABLE;
-			c0_c2_change = C0_C2_CHANGE_DISABLE;
-			csc_enable = v_CSC_ENABLE;
-		}
-	}
-
-	for (i = 0; i < 24; i++)
-		hdmi_writeb(hdmi, HDMI_VIDEO_CSC_COEF + i,
-			    coeff_csc[csc_mode][i]);
-
-	value = v_SOF_DISABLE | csc_enable | v_COLOR_DEPTH_NOT_INDICATED(1);
+	value = v_SOF_DISABLE | v_COLOR_DEPTH_NOT_INDICATED(1);
 	hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL3, value);
-	hdmi_modb(hdmi, HDMI_VIDEO_CONTRL, m_VIDEO_AUTO_CSC |
-		  m_VIDEO_C0_C2_SWAP, v_VIDEO_AUTO_CSC(auto_csc) |
-		  v_VIDEO_C0_C2_SWAP(c0_c2_change));
 
+	hdmi_modb(hdmi, HDMI_VIDEO_CONTRL,
+		  m_VIDEO_AUTO_CSC | m_VIDEO_C0_C2_SWAP,
+		  v_VIDEO_AUTO_CSC(AUTO_CSC_DISABLE) |
+		  v_VIDEO_C0_C2_SWAP(C0_C2_CHANGE_DISABLE));
 	return 0;
 }
 
@@ -425,8 +382,6 @@  static int inno_hdmi_setup(struct inno_hdmi *hdmi,
 	struct drm_display_info *display = &hdmi->connector.display_info;
 	u8 vic = drm_match_cea_mode(mode);
 
-	hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
-
 	if ((vic == 6) || (vic == 7) ||
 	    (vic == 21) || (vic == 22) ||
 	    (vic == 2) || (vic == 3) ||