[PATCH v1 0/4] Rockchip rk3066_hdmi update

[PATCH v1 0/4] Rockchip rk3066_hdmi update

[Johan Jonker]

Update the Rockchip rk3066_hdmi driver in a somewhat similar way
to what is proposed for the inno_hdmi driver.

Johan Jonker (4):
  drm/rockchip: rk3066_hdmi: Remove useless mode_fixup
  drm/rockchip: rk3066_hdmi: Switch encoder hooks to atomic
  drm/rockchip: rk3066_hdmi: Remove useless output format
  drm/rockchip: rk3066_hdmi: Remove unused drm device pointer

 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 66 +++++++-------------------
 1 file changed, 18 insertions(+), 48 deletions(-)

--
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 1/4] drm/rockchip: rk3066_hdmi: Remove useless mode_fixup

[Johan Jonker]

The mode_fixup implementation doesn't do anything, so we can simply
remove it.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index fa6e592e0276..5c269081c691 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -434,14 +434,6 @@ static void rk3066_hdmi_encoder_disable(struct drm_encoder *encoder)
 	rk3066_hdmi_set_power_mode(hdmi, HDMI_SYS_POWER_MODE_A);
 }

-static bool
-rk3066_hdmi_encoder_mode_fixup(struct drm_encoder *encoder,
-			       const struct drm_display_mode *mode,
-			       struct drm_display_mode *adj_mode)
-{
-	return true;
-}
-
 static int
 rk3066_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
 				 struct drm_crtc_state *crtc_state,
@@ -459,7 +451,6 @@ static const
 struct drm_encoder_helper_funcs rk3066_hdmi_encoder_helper_funcs = {
 	.enable       = rk3066_hdmi_encoder_enable,
 	.disable      = rk3066_hdmi_encoder_disable,
-	.mode_fixup   = rk3066_hdmi_encoder_mode_fixup,
 	.mode_set     = rk3066_hdmi_encoder_mode_set,
 	.atomic_check = rk3066_hdmi_encoder_atomic_check,
 };
--
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 2/4] drm/rockchip: rk3066_hdmi: Switch encoder hooks to atomic

[Johan Jonker]

The rk3066_hdmi encoder still uses the non atomic variants
of enable and disable. Convert to their atomic equivalents.
In atomic mode there is no need to save the adjusted mode,
so remove the mode_set function.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 35 +++++++++++++-------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index 5c269081c691..0e7aae341960 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -55,7 +55,6 @@ struct rk3066_hdmi {
 	unsigned int tmdsclk;

 	struct hdmi_data_info hdmi_data;
-	struct drm_display_mode previous_mode;
 };

 static struct rk3066_hdmi *encoder_to_rk3066_hdmi(struct drm_encoder *encoder)
@@ -387,21 +386,21 @@ static int rk3066_hdmi_setup(struct rk3066_hdmi *hdmi,
 	return 0;
 }

-static void
-rk3066_hdmi_encoder_mode_set(struct drm_encoder *encoder,
-			     struct drm_display_mode *mode,
-			     struct drm_display_mode *adj_mode)
+static void rk3066_hdmi_encoder_enable(struct drm_encoder *encoder,
+				       struct drm_atomic_state *state)
 {
 	struct rk3066_hdmi *hdmi = encoder_to_rk3066_hdmi(encoder);
+	struct drm_connector_state *conn_state;
+	struct drm_crtc_state *crtc_state;
+	int mux, val;

-	/* Store the display mode for plugin/DPMS poweron events. */
-	drm_mode_copy(&hdmi->previous_mode, adj_mode);
-}
+	conn_state = drm_atomic_get_new_connector_state(state, &hdmi->connector);
+	if (WARN_ON(!conn_state))
+		return;

-static void rk3066_hdmi_encoder_enable(struct drm_encoder *encoder)
-{
-	struct rk3066_hdmi *hdmi = encoder_to_rk3066_hdmi(encoder);
-	int mux, val;
+	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
+	if (WARN_ON(!crtc_state))
+		return;

 	mux = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder);
 	if (mux)
@@ -414,10 +413,11 @@ static void rk3066_hdmi_encoder_enable(struct drm_encoder *encoder)
 	DRM_DEV_DEBUG(hdmi->dev, "hdmi encoder enable select: vop%s\n",
 		      (mux) ? "1" : "0");

-	rk3066_hdmi_setup(hdmi, &hdmi->previous_mode);
+	rk3066_hdmi_setup(hdmi, &crtc_state->adjusted_mode);
 }

-static void rk3066_hdmi_encoder_disable(struct drm_encoder *encoder)
+static void rk3066_hdmi_encoder_disable(struct drm_encoder *encoder,
+					struct drm_atomic_state *state)
 {
 	struct rk3066_hdmi *hdmi = encoder_to_rk3066_hdmi(encoder);

@@ -449,10 +449,9 @@ rk3066_hdmi_encoder_atomic_check(struct drm_encoder *encoder,

 static const
 struct drm_encoder_helper_funcs rk3066_hdmi_encoder_helper_funcs = {
-	.enable       = rk3066_hdmi_encoder_enable,
-	.disable      = rk3066_hdmi_encoder_disable,
-	.mode_set     = rk3066_hdmi_encoder_mode_set,
-	.atomic_check = rk3066_hdmi_encoder_atomic_check,
+	.atomic_check   = rk3066_hdmi_encoder_atomic_check,
+	.atomic_enable  = rk3066_hdmi_encoder_enable,
+	.atomic_disable = rk3066_hdmi_encoder_disable,
 };

 static enum drm_connector_status
--
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 3/4] drm/rockchip: rk3066_hdmi: Remove useless output format

[Johan Jonker]

The Rk3066 hdmi output format is hard coded to RGB. Remove
all useless code related to colorimetry and enc_out_format.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index 0e7aae341960..f2b1b2faa096 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -23,8 +23,6 @@

 struct hdmi_data_info {
 	int vic; /* The CEA Video ID (VIC) of the current drm display mode. */
-	unsigned int enc_out_format;
-	unsigned int colorimetry;
 };

 struct rk3066_hdmi_i2c {
@@ -200,14 +198,7 @@ static int rk3066_hdmi_config_avi(struct rk3066_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.colorimetry = hdmi->hdmi_data.colorimetry;
+	frame.avi.colorspace = HDMI_COLORSPACE_RGB;
 	frame.avi.scan_mode = HDMI_SCAN_MODE_NONE;

 	return rk3066_hdmi_upload_frame(hdmi, rc, &frame,
@@ -329,15 +320,6 @@ static int rk3066_hdmi_setup(struct rk3066_hdmi *hdmi,
 	struct drm_display_info *display = &hdmi->connector.display_info;

 	hdmi->hdmi_data.vic = drm_match_cea_mode(mode);
-	hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
-
-	if (hdmi->hdmi_data.vic == 6 || hdmi->hdmi_data.vic == 7 ||
-	    hdmi->hdmi_data.vic == 21 || hdmi->hdmi_data.vic == 22 ||
-	    hdmi->hdmi_data.vic == 2 || hdmi->hdmi_data.vic == 3 ||
-	    hdmi->hdmi_data.vic == 17 || hdmi->hdmi_data.vic == 18)
-		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_601;
-	else
-		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;

 	hdmi->tmdsclk = mode->clock * 1000;

--
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 3/4] drm/rockchip: rk3066_hdmi: Remove useless output format

[Heiko Stuebner]

Hi Johan,

Am Donnerstag, 2. November 2023, 14:42:19 CET schrieb Johan Jonker:
> The Rk3066 hdmi output format is hard coded to RGB. Remove
> all useless code related to colorimetry and enc_out_format.
> 
> Signed-off-by: Johan Jonker <jbx6244@gmail.com>

I guess my first question is, is the hardcoding happening just because
of missing functionality in the driver, or does the hardware only
support RGB?


> ---
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c | 20 +-------------------
>  1 file changed, 1 insertion(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> index 0e7aae341960..f2b1b2faa096 100644
> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> @@ -23,8 +23,6 @@
> 
>  struct hdmi_data_info {
>  	int vic; /* The CEA Video ID (VIC) of the current drm display mode. */
> -	unsigned int enc_out_format;
> -	unsigned int colorimetry;
>  };
> 
>  struct rk3066_hdmi_i2c {
> @@ -200,14 +198,7 @@ static int rk3066_hdmi_config_avi(struct rk3066_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.colorimetry = hdmi->hdmi_data.colorimetry;
> +	frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>  	frame.avi.scan_mode = HDMI_SCAN_MODE_NONE;
> 
>  	return rk3066_hdmi_upload_frame(hdmi, rc, &frame,
> @@ -329,15 +320,6 @@ static int rk3066_hdmi_setup(struct rk3066_hdmi *hdmi,
>  	struct drm_display_info *display = &hdmi->connector.display_info;
> 
>  	hdmi->hdmi_data.vic = drm_match_cea_mode(mode);
> -	hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
> -
> -	if (hdmi->hdmi_data.vic == 6 || hdmi->hdmi_data.vic == 7 ||
> -	    hdmi->hdmi_data.vic == 21 || hdmi->hdmi_data.vic == 22 ||
> -	    hdmi->hdmi_data.vic == 2 || hdmi->hdmi_data.vic == 3 ||
> -	    hdmi->hdmi_data.vic == 17 || hdmi->hdmi_data.vic == 18)
> -		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_601;
> -	else
> -		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;

while I can understand the RGB output format, why does the colorimetry
also get removed? This looks like it is dependent on the mode itself
and not the output format?

Thanks
Heiko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 3/4] drm/rockchip: rk3066_hdmi: Remove useless output format

[Johan Jonker]

On 11/20/23 18:06, Heiko Stuebner wrote:
> Hi Johan,
> 
> Am Donnerstag, 2. November 2023, 14:42:19 CET schrieb Johan Jonker:
>> The Rk3066 hdmi output format is hard coded to RGB. Remove
>> all useless code related to colorimetry and enc_out_format.
>>
>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> 

> I guess my first question is, is the hardcoding happening just because
> of missing functionality in the driver, or does the hardware only
> support RGB?

This driver can do so much more..., but is crippled by various causes.
If in need for a full functional rk3066 driver a little bit help/advise/action from other people is needed.

1:
Missing rk3066 TRM HDMI register info.
Could Rockchip (= Sandy Huang) disclose this info to the open source community?

As a way around we can look at older driver code and port to mainline.
More info gives better results.
rk30_hdmi_config_csc() function:
https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/chips/rkpx2/rkpx2_hdmi_hw.c#L315

2:
Could DRM people show us examples for:
- How to advertise to the VOP driver what data formats (RGB, YCBCR) it can send to the HDMI driver or any other Rockchip DRM sub driver other then RGB.
- Advertise EDID data monitor modes RGB444, YCBCR444 and YCBCR422 to the HDMI driver.

https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/rk_hdmi_edid.c#L217C1-L218C41

3:
Advise when what Infoframe is needed for only RGB vs. the rest according to the specification.
https://engineering.purdue.edu/ece477/Archive/2012/Spring/S12-Grp10/Datasheets/CEC_HDMI_Specification.pdf

rk3066 currently only sends avi info. Does it need vsi as well? Can anyone give some clarity here?
inno_hdime sends avi and vsi info.

4:
rk3066_hdmi and inno_hdmi are HDMI 1.4a drivers for DVI and HDMI.
Validated by drm_match_cea_mode() this function only gives us both HDMI + HDMI2 focus, but nothing for old DVI monitors.
How to improve?

5:
Sound support was submitted:
Re: [PATCH v6 2/5] drm: rockchip: add sound support to rk3066 hdmi driver
https://lore.kernel.org/linux-rockchip/48dbe9b7-0aa0-f459-301f-f380e2b7f2f8@gmail.com/

No reply was given (by Heiko or others) on why it wasn't applied or what needs to be improved.

Without reply no improvement.

Johan


> 
> 
>> ---
>>  drivers/gpu/drm/rockchip/rk3066_hdmi.c | 20 +-------------------
>>  1 file changed, 1 insertion(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> index 0e7aae341960..f2b1b2faa096 100644
>> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> @@ -23,8 +23,6 @@
>>
>>  struct hdmi_data_info {
>>  	int vic; /* The CEA Video ID (VIC) of the current drm display mode. */
>> -	unsigned int enc_out_format;
>> -	unsigned int colorimetry;
>>  };
>>
>>  struct rk3066_hdmi_i2c {
>> @@ -200,14 +198,7 @@ static int rk3066_hdmi_config_avi(struct rk3066_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.colorimetry = hdmi->hdmi_data.colorimetry;
>> +	frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>>  	frame.avi.scan_mode = HDMI_SCAN_MODE_NONE;
>>
>>  	return rk3066_hdmi_upload_frame(hdmi, rc, &frame,
>> @@ -329,15 +320,6 @@ static int rk3066_hdmi_setup(struct rk3066_hdmi *hdmi,
>>  	struct drm_display_info *display = &hdmi->connector.display_info;
>>
>>  	hdmi->hdmi_data.vic = drm_match_cea_mode(mode);
>> -	hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
>> -
>> -	if (hdmi->hdmi_data.vic == 6 || hdmi->hdmi_data.vic == 7 ||
>> -	    hdmi->hdmi_data.vic == 21 || hdmi->hdmi_data.vic == 22 ||
>> -	    hdmi->hdmi_data.vic == 2 || hdmi->hdmi_data.vic == 3 ||
>> -	    hdmi->hdmi_data.vic == 17 || hdmi->hdmi_data.vic == 18)
>> -		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_601;
>> -	else
>> -		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;
> 

> while I can understand the RGB output format, why does the colorimetry
> also get removed? This looks like it is dependent on the mode itself
> and not the output format?

From the old driver these conditions apply whether csc is needed.
https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/chips/rkpx2/rkpx2_hdmi_hw.c#L320C1-L324C3

	if( ((vpara->input_color == VIDEO_INPUT_COLOR_RGB) && (vpara->output_color == VIDEO_OUTPUT_RGB444)) ||
		((vpara->input_color == VIDEO_INPUT_COLOR_YCBCR) && (vpara->output_color != VIDEO_OUTPUT_RGB444) ))
	{
		return;
	}

> 
> Thanks
> Heiko
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 3/4] drm/rockchip: rk3066_hdmi: Remove useless output format

[Andy Yan]

Hi Johan:

some information bellow hope can help a bit.

On 11/23/23 20:54, Johan Jonker wrote:
>
> On 11/20/23 18:06, Heiko Stuebner wrote:
>> Hi Johan,
>>
>> Am Donnerstag, 2. November 2023, 14:42:19 CET schrieb Johan Jonker:
>>> The Rk3066 hdmi output format is hard coded to RGB. Remove
>>> all useless code related to colorimetry and enc_out_format.
>>>
>>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>> I guess my first question is, is the hardcoding happening just because
>> of missing functionality in the driver, or does the hardware only
>> support RGB?
> This driver can do so much more..., but is crippled by various causes.
> If in need for a full functional rk3066 driver a little bit help/advise/action from other people is needed.
>
> 1:
> Missing rk3066 TRM HDMI register info.
> Could Rockchip (= Sandy Huang) disclose this info to the open source community?

The  HDMI on rk3066 is from a IP vendor, so the detail of this IP are not even

include in the TRM.

As it is a chip which is more than 10 yeas old, I contacted the author of the bsp

driver, got some information bellow:

This IP is almost the same with sh_mobile_hdmi, unfortunately, SH-Mobile HDMI drivers

is removed out of mainline in 2015[0], but with a quick look at it, the register definition

is the same as rk3066 hdmi and with more detail description.


[0]https://lkml.kernel.org/stable/20191122100825.930987859@linuxfoundation.org/

>
> As a way around we can look at older driver code and port to mainline.
> More info gives better results.
> rk30_hdmi_config_csc() function:
> https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/chips/rkpx2/rkpx2_hdmi_hw.c#L315
>
> 2:
> Could DRM people show us examples for:
> - How to advertise to the VOP driver what data formats (RGB, YCBCR) it can send to the HDMI driver or any other Rockchip DRM sub driver other then RGB.
> - Advertise EDID data monitor modes RGB444, YCBCR444 and YCBCR422 to the HDMI driver.
>
> https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/rk_hdmi_edid.c#L217C1-L218C41


RK3066 vop can only output RGB full range to HDMI, so the full to limit rgb to yuv conversion is done by rk30_hdmi_config_csc.

>
> 3:
> Advise when what Infoframe is needed for only RGB vs. the rest according to the specification.
> https://engineering.purdue.edu/ece477/Archive/2012/Spring/S12-Grp10/Datasheets/CEC_HDMI_Specification.pdf
>
> rk3066 currently only sends avi info. Does it need vsi as well? Can anyone give some clarity here?
> inno_hdime sends avi and vsi info.

vsi is used for 3d and hdmi 1.4 format(4K24/25/30, not support by rk3066), or vendor specific data like timecode, dolby,

so as a basic function, we don't need it.

>
> 4:
> rk3066_hdmi and inno_hdmi are HDMI 1.4a drivers for DVI and HDMI.
> Validated by drm_match_cea_mode() this function only gives us both HDMI + HDMI2 focus, but nothing for old DVI monitors.
> How to improve?
>
> 5:
> Sound support was submitted:
> Re: [PATCH v6 2/5] drm: rockchip: add sound support to rk3066 hdmi driver
> https://lore.kernel.org/linux-rockchip/48dbe9b7-0aa0-f459-301f-f380e2b7f2f8@gmail.com/
>
> No reply was given (by Heiko or others) on why it wasn't applied or what needs to be improved.
>
> Without reply no improvement.
>
> Johan
>
>
>>
>>> ---
>>>   drivers/gpu/drm/rockchip/rk3066_hdmi.c | 20 +-------------------
>>>   1 file changed, 1 insertion(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>>> index 0e7aae341960..f2b1b2faa096 100644
>>> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>>> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>>> @@ -23,8 +23,6 @@
>>>
>>>   struct hdmi_data_info {
>>>   	int vic; /* The CEA Video ID (VIC) of the current drm display mode. */
>>> -	unsigned int enc_out_format;
>>> -	unsigned int colorimetry;
>>>   };
>>>
>>>   struct rk3066_hdmi_i2c {
>>> @@ -200,14 +198,7 @@ static int rk3066_hdmi_config_avi(struct rk3066_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.colorimetry = hdmi->hdmi_data.colorimetry;
>>> +	frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>>>   	frame.avi.scan_mode = HDMI_SCAN_MODE_NONE;
>>>
>>>   	return rk3066_hdmi_upload_frame(hdmi, rc, &frame,
>>> @@ -329,15 +320,6 @@ static int rk3066_hdmi_setup(struct rk3066_hdmi *hdmi,
>>>   	struct drm_display_info *display = &hdmi->connector.display_info;
>>>
>>>   	hdmi->hdmi_data.vic = drm_match_cea_mode(mode);
>>> -	hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
>>> -
>>> -	if (hdmi->hdmi_data.vic == 6 || hdmi->hdmi_data.vic == 7 ||
>>> -	    hdmi->hdmi_data.vic == 21 || hdmi->hdmi_data.vic == 22 ||
>>> -	    hdmi->hdmi_data.vic == 2 || hdmi->hdmi_data.vic == 3 ||
>>> -	    hdmi->hdmi_data.vic == 17 || hdmi->hdmi_data.vic == 18)
>>> -		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_601;
>>> -	else
>>> -		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;
>> while I can understand the RGB output format, why does the colorimetry
>> also get removed? This looks like it is dependent on the mode itself
>> and not the output format?
> >From the old driver these conditions apply whether csc is needed.
> https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/chips/rkpx2/rkpx2_hdmi_hw.c#L320C1-L324C3
>
> 	if( ((vpara->input_color == VIDEO_INPUT_COLOR_RGB) && (vpara->output_color == VIDEO_OUTPUT_RGB444)) ||
> 		((vpara->input_color == VIDEO_INPUT_COLOR_YCBCR) && (vpara->output_color != VIDEO_OUTPUT_RGB444) ))
> 	{
> 		return;
> 	}
>
>> Thanks
>> Heiko
>>
>>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 3/4] drm/rockchip: rk3066_hdmi: Remove useless output format

[Heiko Stübner]

Hi Johan,

Am Donnerstag, 23. November 2023, 13:54:28 CET schrieb Johan Jonker:
> 
> On 11/20/23 18:06, Heiko Stuebner wrote:
> > Hi Johan,
> > 
> > Am Donnerstag, 2. November 2023, 14:42:19 CET schrieb Johan Jonker:
> >> The Rk3066 hdmi output format is hard coded to RGB. Remove
> >> all useless code related to colorimetry and enc_out_format.
> >>
> >> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> > 
> 
> > I guess my first question is, is the hardcoding happening just because
> > of missing functionality in the driver, or does the hardware only
> > support RGB?
> 
> This driver can do so much more..., but is crippled by various causes.
> If in need for a full functional rk3066 driver a little bit help/advise/action from other people is needed.

Part of me wants to have fully working drivers, but on the other hand,
both the rk3066-hdmi and also the inno-hdmi drivers are sort of one-off
drivers used by rk3066 and rk3036 (inno-hdmi) and most likely won't see
new SoCs using them in the future.


So I guess after thinking more about this, I should probably just apply
your patch to simplify the code and if by some magical happenings in
future someone really wants to spend time on either one of these drivers
they can always use "git revert" to bring back the old code?


Heiko


> 1:
> Missing rk3066 TRM HDMI register info.
> Could Rockchip (= Sandy Huang) disclose this info to the open source community?
> 
> As a way around we can look at older driver code and port to mainline.
> More info gives better results.
> rk30_hdmi_config_csc() function:
> https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/chips/rkpx2/rkpx2_hdmi_hw.c#L315
> 
> 2:
> Could DRM people show us examples for:
> - How to advertise to the VOP driver what data formats (RGB, YCBCR) it can send to the HDMI driver or any other Rockchip DRM sub driver other then RGB.
> - Advertise EDID data monitor modes RGB444, YCBCR444 and YCBCR422 to the HDMI driver.
> 
> https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/rk_hdmi_edid.c#L217C1-L218C41
> 
> 3:
> Advise when what Infoframe is needed for only RGB vs. the rest according to the specification.
> https://engineering.purdue.edu/ece477/Archive/2012/Spring/S12-Grp10/Datasheets/CEC_HDMI_Specification.pdf
> 
> rk3066 currently only sends avi info. Does it need vsi as well? Can anyone give some clarity here?
> inno_hdime sends avi and vsi info.
> 
> 4:
> rk3066_hdmi and inno_hdmi are HDMI 1.4a drivers for DVI and HDMI.
> Validated by drm_match_cea_mode() this function only gives us both HDMI + HDMI2 focus, but nothing for old DVI monitors.
> How to improve?
> 
> 5:
> Sound support was submitted:
> Re: [PATCH v6 2/5] drm: rockchip: add sound support to rk3066 hdmi driver
> https://lore.kernel.org/linux-rockchip/48dbe9b7-0aa0-f459-301f-f380e2b7f2f8@gmail.com/
> 
> No reply was given (by Heiko or others) on why it wasn't applied or what needs to be improved.
> 
> Without reply no improvement.
> 
> Johan
> 
> 
> > 
> > 
> >> ---
> >>  drivers/gpu/drm/rockchip/rk3066_hdmi.c | 20 +-------------------
> >>  1 file changed, 1 insertion(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> >> index 0e7aae341960..f2b1b2faa096 100644
> >> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> >> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> >> @@ -23,8 +23,6 @@
> >>
> >>  struct hdmi_data_info {
> >>  	int vic; /* The CEA Video ID (VIC) of the current drm display mode. */
> >> -	unsigned int enc_out_format;
> >> -	unsigned int colorimetry;
> >>  };
> >>
> >>  struct rk3066_hdmi_i2c {
> >> @@ -200,14 +198,7 @@ static int rk3066_hdmi_config_avi(struct rk3066_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.colorimetry = hdmi->hdmi_data.colorimetry;
> >> +	frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> >>  	frame.avi.scan_mode = HDMI_SCAN_MODE_NONE;
> >>
> >>  	return rk3066_hdmi_upload_frame(hdmi, rc, &frame,
> >> @@ -329,15 +320,6 @@ static int rk3066_hdmi_setup(struct rk3066_hdmi *hdmi,
> >>  	struct drm_display_info *display = &hdmi->connector.display_info;
> >>
> >>  	hdmi->hdmi_data.vic = drm_match_cea_mode(mode);
> >> -	hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
> >> -
> >> -	if (hdmi->hdmi_data.vic == 6 || hdmi->hdmi_data.vic == 7 ||
> >> -	    hdmi->hdmi_data.vic == 21 || hdmi->hdmi_data.vic == 22 ||
> >> -	    hdmi->hdmi_data.vic == 2 || hdmi->hdmi_data.vic == 3 ||
> >> -	    hdmi->hdmi_data.vic == 17 || hdmi->hdmi_data.vic == 18)
> >> -		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_601;
> >> -	else
> >> -		hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;
> > 
> 
> > while I can understand the RGB output format, why does the colorimetry
> > also get removed? This looks like it is dependent on the mode itself
> > and not the output format?
> 
> From the old driver these conditions apply whether csc is needed.
> https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/chips/rkpx2/rkpx2_hdmi_hw.c#L320C1-L324C3
> 
> 	if( ((vpara->input_color == VIDEO_INPUT_COLOR_RGB) && (vpara->output_color == VIDEO_OUTPUT_RGB444)) ||
> 		((vpara->input_color == VIDEO_INPUT_COLOR_YCBCR) && (vpara->output_color != VIDEO_OUTPUT_RGB444) ))
> 	{
> 		return;
> 	}
> 
> > 
> > Thanks
> > Heiko
> > 
> > 
> 





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 4/4] drm/rockchip: rk3066_hdmi: Remove unused drm device pointer

[Johan Jonker]

The drm_dev field in the rk3066_hdmi struct stores a pointer to the DRM
device but is never used anywhere in the driver. Let's remove it.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index f2b1b2faa096..c51520ec58d2 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -38,7 +38,6 @@ struct rk3066_hdmi_i2c {

 struct rk3066_hdmi {
 	struct device *dev;
-	struct drm_device *drm_dev;
 	struct regmap *grf_regmap;
 	int irq;
 	struct clk *hclk;
@@ -734,7 +733,6 @@ static int rk3066_hdmi_bind(struct device *dev, struct device *master,
 		return -ENOMEM;

 	hdmi->dev = dev;
-	hdmi->drm_dev = drm;
 	hdmi->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(hdmi->regs))
 		return PTR_ERR(hdmi->regs);
--
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: (subset) [PATCH v1 0/4] Rockchip rk3066_hdmi update

[Heiko Stuebner]

On Thu, 2 Nov 2023 14:40:13 +0100, Johan Jonker wrote:
> Update the Rockchip rk3066_hdmi driver in a somewhat similar way
> to what is proposed for the inno_hdmi driver.
> 
> Johan Jonker (4):
>   drm/rockchip: rk3066_hdmi: Remove useless mode_fixup
>   drm/rockchip: rk3066_hdmi: Switch encoder hooks to atomic
>   drm/rockchip: rk3066_hdmi: Remove useless output format
>   drm/rockchip: rk3066_hdmi: Remove unused drm device pointer
> 
> [...]

Applied, thanks!

[1/4] drm/rockchip: rk3066_hdmi: Remove useless mode_fixup
      commit: 1044f4a31734eef000f42cdaaf35bb2f76286be5
[2/4] drm/rockchip: rk3066_hdmi: Switch encoder hooks to atomic
      commit: ae3436a5e7c2ef4f92938133bd99f92fc47ea34e

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel