[PATCH v2 1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

[PATCH v2 1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

[Aditya Swarup]

From: Clinton Taylor <Clinton.A.Taylor@intel.com>

v2: Fix commit msg to reflect why issue occurs(Jani)
Set GCP_COLOR_INDICATION only when we set 10/12 bit deep color.

Changing settings from 10/12 bit deep color to 8 bit(& vice versa)
doesn't work correctly using xrandr max bpc property. When we
connect a monitor which supports deep color, the highest deep color
setting is selected; which sets GCP_COLOR_INDICATION. When we change
the setting to 8 bit color, we still set GCP_COLOR_INDICATION which
doesn't allow the switch back to 8 bit color.

Signed-off-by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 5ccb305a6e1c..4760462f84ca 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -963,7 +963,9 @@ static void intel_hdmi_compute_gcp_infoframe(struct intel_encoder *encoder,
 		intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GENERAL_CONTROL);
 
 	/* Indicate color depth whenever the sink supports deep color */
-	if (hdmi_sink_is_deep_color(conn_state))
+
+	if (hdmi_sink_is_deep_color(conn_state) &&
+	    (crtc_state->pipe_bpp > 24))
 		crtc_state->infoframes.gcp |= GCP_COLOR_INDICATION;
 
 	/* Enable default_phase whenever the display mode is suitably aligned */
@@ -2260,6 +2262,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
 	int clock_10bpc = clock_8bpc * 5 / 4;
 	int clock_12bpc = clock_8bpc * 3 / 2;
+	int dc_clock = clock_12bpc;
 	int desired_bpp;
 	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
 
@@ -2314,22 +2317,18 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 	 * Note that g4x/vlv don't support 12bpc hdmi outputs. We also need
 	 * to check that the higher clock still fits within limits.
 	 */
-	if (hdmi_deep_color_possible(pipe_config, 12) &&
-	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc,
+	if (pipe_config->pipe_bpp == 30)
+		dc_clock = clock_10bpc;
+
+	if (hdmi_deep_color_possible(pipe_config, pipe_config->pipe_bpp / 3) &&
+	    hdmi_port_clock_valid(intel_hdmi, dc_clock,
 				  true, force_dvi) == MODE_OK) {
-		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
-		desired_bpp = 12*3;
-
-		/* Need to adjust the port link by 1.5x for 12bpc. */
-		pipe_config->port_clock = clock_12bpc;
-	} else if (hdmi_deep_color_possible(pipe_config, 10) &&
-		   hdmi_port_clock_valid(intel_hdmi, clock_10bpc,
-					 true, force_dvi) == MODE_OK) {
-		DRM_DEBUG_KMS("picking bpc to 10 for HDMI output\n");
-		desired_bpp = 10 * 3;
-
-		/* Need to adjust the port link by 1.25x for 10bpc. */
-		pipe_config->port_clock = clock_10bpc;
+		DRM_DEBUG_KMS("picking bpc to %d for HDMI output\n",
+				 pipe_config->pipe_bpp / 3);
+		desired_bpp = pipe_config->pipe_bpp;
+
+		/* Need to adjust the port link dc modes. */
+		pipe_config->port_clock = dc_clock;
 	} else {
 		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
 		desired_bpp = 8*3;
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color

[Aditya Swarup]

Adding N & CTS values for 10/12 bit deep color from Appendix C
table in HDMI 2.0 spec. The correct values for N is not chosen
automatically by hardware for deep color modes.

v2: Remove redundant code and make it generic.(Jani)

Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 82 +++++++++++++++++++++++++-----
 1 file changed, 69 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 502b57ce72ab..ad53b04fa5a2 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -70,6 +70,13 @@ struct dp_aud_n_m {
 	u16 n;
 };
 
+struct hdmi_aud_ncts_table {
+	int sample_rate;
+	int clock;
+	int n;
+	int cts;
+};
+
 /* Values according to DP 1.4 Table 2-104 */
 static const struct dp_aud_n_m dp_aud_n_m[] = {
 	{ 32000, LC_162M, 1024, 10125 },
@@ -146,12 +153,7 @@ static const struct {
 #define TMDS_594M 594000
 #define TMDS_593M 593407
 
-static const struct {
-	int sample_rate;
-	int clock;
-	int n;
-	int cts;
-} hdmi_aud_ncts[] = {
+static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
 	{ 32000, TMDS_296M, 5824, 421875 },
 	{ 32000, TMDS_297M, 3072, 222750 },
 	{ 32000, TMDS_593M, 5824, 843750 },
@@ -182,6 +184,49 @@ static const struct {
 	{ 192000, TMDS_594M, 24576, 594000 },
 };
 
+/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
+/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
+#define TMDS_371M 371250
+#define TMDS_370M 370878
+
+static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
+	{ 32000, TMDS_370M, 5824, 527344 },
+	{ 32000, TMDS_371M, 6144, 556875 },
+	{ 44100, TMDS_370M, 8918, 585938 },
+	{ 44100, TMDS_371M, 4704, 309375 },
+	{ 88200, TMDS_370M, 17836, 585938 },
+	{ 88200, TMDS_371M, 9408, 309375 },
+	{ 176400, TMDS_370M, 35672, 585938 },
+	{ 176400, TMDS_371M, 18816, 309375 },
+	{ 48000, TMDS_370M, 11648, 703125 },
+	{ 48000, TMDS_371M, 5120, 309375 },
+	{ 96000, TMDS_370M, 23296, 703125 },
+	{ 96000, TMDS_371M, 10240, 309375 },
+	{ 192000, TMDS_370M, 46592, 703125 },
+	{ 192000, TMDS_371M, 20480, 309375 },
+};
+
+/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
+#define TMDS_445_5M 445500
+#define TMDS_445M 445054
+
+static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
+	{ 32000, TMDS_445M, 5824, 632813 },
+	{ 32000, TMDS_445_5M, 4096, 445500 },
+	{ 44100, TMDS_445M, 8918, 703125 },
+	{ 44100, TMDS_445_5M, 4704, 371250 },
+	{ 88200, TMDS_445M, 17836, 703125 },
+	{ 88200, TMDS_445_5M, 9408, 371250 },
+	{ 176400, TMDS_445M, 35672, 703125 },
+	{ 176400, TMDS_445_5M, 18816, 371250 },
+	{ 48000, TMDS_445M, 5824, 421875 },
+	{ 48000, TMDS_445_5M, 5120, 371250 },
+	{ 96000, TMDS_445M, 11648, 421875 },
+	{ 96000, TMDS_445_5M, 10240, 371250 },
+	{ 192000, TMDS_445M, 23296, 421875 },
+	{ 192000, TMDS_445_5M, 20480, 371250 },
+};
+
 /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
 static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_state)
 {
@@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_sta
 static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
 				   int rate)
 {
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->base.adjusted_mode;
-	int i;
+	const struct hdmi_aud_ncts_table *hdmi_ncts_table;
+	int i, size = 0;
+
+	if (crtc_state->pipe_bpp == 36) {
+		hdmi_ncts_table = hdmi_aud_ncts_36bpp;
+		size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
+	} else if (crtc_state->pipe_bpp == 30) {
+		hdmi_ncts_table = hdmi_aud_ncts_30bpp;
+		size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
+	} else {
+		hdmi_ncts_table = hdmi_aud_ncts_24bpp;
+		size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
+	}
 
-	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
-		if (rate == hdmi_aud_ncts[i].sample_rate &&
-		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
-			return hdmi_aud_ncts[i].n;
+	for (i = 0;  i < size; i++) {
+		if (rate == hdmi_ncts_table[i].sample_rate &&
+		    crtc_state->port_clock == hdmi_ncts_table[i].clock) {
+			return hdmi_ncts_table[i].n;
 		}
 	}
+
 	return 0;
 }
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color

[Aditya Swarup]

On Tue, Apr 02, 2019 at 05:14:40AM -0700, Aditya Swarup wrote:
> Adding N & CTS values for 10/12 bit deep color from Appendix C
> table in HDMI 2.0 spec. The correct values for N is not chosen
> automatically by hardware for deep color modes.
> 
> v2: Remove redundant code and make it generic.(Jani)
> 
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 82 +++++++++++++++++++++++++-----
>  1 file changed, 69 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 502b57ce72ab..ad53b04fa5a2 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -70,6 +70,13 @@ struct dp_aud_n_m {
>  	u16 n;
>  };
>  
> +struct hdmi_aud_ncts_table {
> +	int sample_rate;
> +	int clock;
> +	int n;
> +	int cts;
> +};
> +
>  /* Values according to DP 1.4 Table 2-104 */
>  static const struct dp_aud_n_m dp_aud_n_m[] = {
>  	{ 32000, LC_162M, 1024, 10125 },
> @@ -146,12 +153,7 @@ static const struct {
>  #define TMDS_594M 594000
>  #define TMDS_593M 593407
>  
> -static const struct {
> -	int sample_rate;
> -	int clock;
> -	int n;
> -	int cts;
> -} hdmi_aud_ncts[] = {
> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
>  	{ 32000, TMDS_296M, 5824, 421875 },
>  	{ 32000, TMDS_297M, 3072, 222750 },
>  	{ 32000, TMDS_593M, 5824, 843750 },
> @@ -182,6 +184,49 @@ static const struct {
>  	{ 192000, TMDS_594M, 24576, 594000 },
>  };
>  
> +/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
> +/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
> +#define TMDS_371M 371250
> +#define TMDS_370M 370878
> +
> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
> +	{ 32000, TMDS_370M, 5824, 527344 },
> +	{ 32000, TMDS_371M, 6144, 556875 },
> +	{ 44100, TMDS_370M, 8918, 585938 },
> +	{ 44100, TMDS_371M, 4704, 309375 },
> +	{ 88200, TMDS_370M, 17836, 585938 },
> +	{ 88200, TMDS_371M, 9408, 309375 },
> +	{ 176400, TMDS_370M, 35672, 585938 },
> +	{ 176400, TMDS_371M, 18816, 309375 },
> +	{ 48000, TMDS_370M, 11648, 703125 },
> +	{ 48000, TMDS_371M, 5120, 309375 },
> +	{ 96000, TMDS_370M, 23296, 703125 },
> +	{ 96000, TMDS_371M, 10240, 309375 },
> +	{ 192000, TMDS_370M, 46592, 703125 },
> +	{ 192000, TMDS_371M, 20480, 309375 },
> +};
> +
> +/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
> +#define TMDS_445_5M 445500
> +#define TMDS_445M 445054
> +
> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
> +	{ 32000, TMDS_445M, 5824, 632813 },
> +	{ 32000, TMDS_445_5M, 4096, 445500 },
> +	{ 44100, TMDS_445M, 8918, 703125 },
> +	{ 44100, TMDS_445_5M, 4704, 371250 },
> +	{ 88200, TMDS_445M, 17836, 703125 },
> +	{ 88200, TMDS_445_5M, 9408, 371250 },
> +	{ 176400, TMDS_445M, 35672, 703125 },
> +	{ 176400, TMDS_445_5M, 18816, 371250 },
> +	{ 48000, TMDS_445M, 5824, 421875 },
> +	{ 48000, TMDS_445_5M, 5120, 371250 },
> +	{ 96000, TMDS_445M, 11648, 421875 },
> +	{ 96000, TMDS_445_5M, 10240, 371250 },
> +	{ 192000, TMDS_445M, 23296, 421875 },
> +	{ 192000, TMDS_445_5M, 20480, 371250 },
> +};
> +
>  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>  static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_state)
>  {
> @@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_sta
>  static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
>  				   int rate)
>  {
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->base.adjusted_mode;
> -	int i;
> +	const struct hdmi_aud_ncts_table *hdmi_ncts_table;
> +	int i, size = 0;
> +
> +	if (crtc_state->pipe_bpp == 36) {
> +		hdmi_ncts_table = hdmi_aud_ncts_36bpp;
> +		size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
> +	} else if (crtc_state->pipe_bpp == 30) {
> +		hdmi_ncts_table = hdmi_aud_ncts_30bpp;
> +		size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
> +	} else {
> +		hdmi_ncts_table = hdmi_aud_ncts_24bpp;
> +		size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
> +	}
>  
> -	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
> -		if (rate == hdmi_aud_ncts[i].sample_rate &&
> -		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
> -			return hdmi_aud_ncts[i].n;
> +	for (i = 0;  i < size; i++) {
> +		if (rate == hdmi_ncts_table[i].sample_rate &&
> +		    crtc_state->port_clock == hdmi_ncts_table[i].clock) {
> +			return hdmi_ncts_table[i].n;
>  		}
>  	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.17.1
> 
Jani
Do you want me to change anything in this patch?

Regards,
Aditya Swarup
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color

[Jani Nikula]

On Thu, 04 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
> On Tue, Apr 02, 2019 at 05:14:40AM -0700, Aditya Swarup wrote:
>> Adding N & CTS values for 10/12 bit deep color from Appendix C
>> table in HDMI 2.0 spec. The correct values for N is not chosen
>> automatically by hardware for deep color modes.
>> 
>> v2: Remove redundant code and make it generic.(Jani)
>> 
>> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_audio.c | 82 +++++++++++++++++++++++++-----
>>  1 file changed, 69 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> index 502b57ce72ab..ad53b04fa5a2 100644
>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> @@ -70,6 +70,13 @@ struct dp_aud_n_m {
>>  	u16 n;
>>  };
>>  
>> +struct hdmi_aud_ncts_table {
>> +	int sample_rate;
>> +	int clock;
>> +	int n;
>> +	int cts;
>> +};
>> +
>>  /* Values according to DP 1.4 Table 2-104 */
>>  static const struct dp_aud_n_m dp_aud_n_m[] = {
>>  	{ 32000, LC_162M, 1024, 10125 },
>> @@ -146,12 +153,7 @@ static const struct {
>>  #define TMDS_594M 594000
>>  #define TMDS_593M 593407
>>  
>> -static const struct {
>> -	int sample_rate;
>> -	int clock;
>> -	int n;
>> -	int cts;
>> -} hdmi_aud_ncts[] = {
>> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
>>  	{ 32000, TMDS_296M, 5824, 421875 },
>>  	{ 32000, TMDS_297M, 3072, 222750 },
>>  	{ 32000, TMDS_593M, 5824, 843750 },
>> @@ -182,6 +184,49 @@ static const struct {
>>  	{ 192000, TMDS_594M, 24576, 594000 },
>>  };
>>  
>> +/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
>> +/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
>> +#define TMDS_371M 371250
>> +#define TMDS_370M 370878
>> +
>> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
>> +	{ 32000, TMDS_370M, 5824, 527344 },
>> +	{ 32000, TMDS_371M, 6144, 556875 },
>> +	{ 44100, TMDS_370M, 8918, 585938 },
>> +	{ 44100, TMDS_371M, 4704, 309375 },
>> +	{ 88200, TMDS_370M, 17836, 585938 },
>> +	{ 88200, TMDS_371M, 9408, 309375 },
>> +	{ 176400, TMDS_370M, 35672, 585938 },
>> +	{ 176400, TMDS_371M, 18816, 309375 },
>> +	{ 48000, TMDS_370M, 11648, 703125 },
>> +	{ 48000, TMDS_371M, 5120, 309375 },
>> +	{ 96000, TMDS_370M, 23296, 703125 },
>> +	{ 96000, TMDS_371M, 10240, 309375 },
>> +	{ 192000, TMDS_370M, 46592, 703125 },
>> +	{ 192000, TMDS_371M, 20480, 309375 },
>> +};
>> +
>> +/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
>> +#define TMDS_445_5M 445500
>> +#define TMDS_445M 445054
>> +
>> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
>> +	{ 32000, TMDS_445M, 5824, 632813 },
>> +	{ 32000, TMDS_445_5M, 4096, 445500 },
>> +	{ 44100, TMDS_445M, 8918, 703125 },
>> +	{ 44100, TMDS_445_5M, 4704, 371250 },
>> +	{ 88200, TMDS_445M, 17836, 703125 },
>> +	{ 88200, TMDS_445_5M, 9408, 371250 },
>> +	{ 176400, TMDS_445M, 35672, 703125 },
>> +	{ 176400, TMDS_445_5M, 18816, 371250 },
>> +	{ 48000, TMDS_445M, 5824, 421875 },
>> +	{ 48000, TMDS_445_5M, 5120, 371250 },
>> +	{ 96000, TMDS_445M, 11648, 421875 },
>> +	{ 96000, TMDS_445_5M, 10240, 371250 },
>> +	{ 192000, TMDS_445M, 23296, 421875 },
>> +	{ 192000, TMDS_445_5M, 20480, 371250 },
>> +};
>> +
>>  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>>  static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_state)
>>  {
>> @@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_sta
>>  static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
>>  				   int rate)
>>  {
>> -	const struct drm_display_mode *adjusted_mode =
>> -		&crtc_state->base.adjusted_mode;
>> -	int i;
>> +	const struct hdmi_aud_ncts_table *hdmi_ncts_table;
>> +	int i, size = 0;
>> +
>> +	if (crtc_state->pipe_bpp == 36) {
>> +		hdmi_ncts_table = hdmi_aud_ncts_36bpp;
>> +		size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
>> +	} else if (crtc_state->pipe_bpp == 30) {
>> +		hdmi_ncts_table = hdmi_aud_ncts_30bpp;
>> +		size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
>> +	} else {
>> +		hdmi_ncts_table = hdmi_aud_ncts_24bpp;
>> +		size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
>> +	}
>>  
>> -	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
>> -		if (rate == hdmi_aud_ncts[i].sample_rate &&
>> -		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
>> -			return hdmi_aud_ncts[i].n;
>> +	for (i = 0;  i < size; i++) {
>> +		if (rate == hdmi_ncts_table[i].sample_rate &&
>> +		    crtc_state->port_clock == hdmi_ncts_table[i].clock) {
>> +			return hdmi_ncts_table[i].n;
>>  		}
>>  	}
>> +
>>  	return 0;
>>  }
>>  
>> -- 
>> 2.17.1
>> 
> Jani
> Do you want me to change anything in this patch?

Hey, please don't drop things on the floor if you don't hear from me!

I didn't look at the spec, but it seems odd to me that the bpp would
limit us to specific port clocks. Is that really so? What if you have,
say, TMDS_594M and 36 bpp, this wouldn't find the params.

BR,
Jani.


>
> Regards,
> Aditya Swarup

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color

[Clinton Taylor]

On 4/30/19 2:25 AM, Jani Nikula wrote:
> On Thu, 04 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
>> On Tue, Apr 02, 2019 at 05:14:40AM -0700, Aditya Swarup wrote:
>>> Adding N & CTS values for 10/12 bit deep color from Appendix C
>>> table in HDMI 2.0 spec. The correct values for N is not chosen
>>> automatically by hardware for deep color modes.
>>>
>>> v2: Remove redundant code and make it generic.(Jani)
>>>
>>> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>>> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_audio.c | 82 +++++++++++++++++++++++++-----
>>>   1 file changed, 69 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>> index 502b57ce72ab..ad53b04fa5a2 100644
>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>> @@ -70,6 +70,13 @@ struct dp_aud_n_m {
>>>   	u16 n;
>>>   };
>>>   
>>> +struct hdmi_aud_ncts_table {
>>> +	int sample_rate;
>>> +	int clock;
>>> +	int n;
>>> +	int cts;
>>> +};
>>> +
>>>   /* Values according to DP 1.4 Table 2-104 */
>>>   static const struct dp_aud_n_m dp_aud_n_m[] = {
>>>   	{ 32000, LC_162M, 1024, 10125 },
>>> @@ -146,12 +153,7 @@ static const struct {
>>>   #define TMDS_594M 594000
>>>   #define TMDS_593M 593407
>>>   
>>> -static const struct {
>>> -	int sample_rate;
>>> -	int clock;
>>> -	int n;
>>> -	int cts;
>>> -} hdmi_aud_ncts[] = {
>>> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
>>>   	{ 32000, TMDS_296M, 5824, 421875 },
>>>   	{ 32000, TMDS_297M, 3072, 222750 },
>>>   	{ 32000, TMDS_593M, 5824, 843750 },
>>> @@ -182,6 +184,49 @@ static const struct {
>>>   	{ 192000, TMDS_594M, 24576, 594000 },
>>>   };
>>>   
>>> +/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
>>> +/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
>>> +#define TMDS_371M 371250
>>> +#define TMDS_370M 370878
>>> +
>>> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
>>> +	{ 32000, TMDS_370M, 5824, 527344 },
>>> +	{ 32000, TMDS_371M, 6144, 556875 },
>>> +	{ 44100, TMDS_370M, 8918, 585938 },
>>> +	{ 44100, TMDS_371M, 4704, 309375 },
>>> +	{ 88200, TMDS_370M, 17836, 585938 },
>>> +	{ 88200, TMDS_371M, 9408, 309375 },
>>> +	{ 176400, TMDS_370M, 35672, 585938 },
>>> +	{ 176400, TMDS_371M, 18816, 309375 },
>>> +	{ 48000, TMDS_370M, 11648, 703125 },
>>> +	{ 48000, TMDS_371M, 5120, 309375 },
>>> +	{ 96000, TMDS_370M, 23296, 703125 },
>>> +	{ 96000, TMDS_371M, 10240, 309375 },
>>> +	{ 192000, TMDS_370M, 46592, 703125 },
>>> +	{ 192000, TMDS_371M, 20480, 309375 },
>>> +};
>>> +
>>> +/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
>>> +#define TMDS_445_5M 445500
>>> +#define TMDS_445M 445054
>>> +
>>> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
>>> +	{ 32000, TMDS_445M, 5824, 632813 },
>>> +	{ 32000, TMDS_445_5M, 4096, 445500 },
>>> +	{ 44100, TMDS_445M, 8918, 703125 },
>>> +	{ 44100, TMDS_445_5M, 4704, 371250 },
>>> +	{ 88200, TMDS_445M, 17836, 703125 },
>>> +	{ 88200, TMDS_445_5M, 9408, 371250 },
>>> +	{ 176400, TMDS_445M, 35672, 703125 },
>>> +	{ 176400, TMDS_445_5M, 18816, 371250 },
>>> +	{ 48000, TMDS_445M, 5824, 421875 },
>>> +	{ 48000, TMDS_445_5M, 5120, 371250 },
>>> +	{ 96000, TMDS_445M, 11648, 421875 },
>>> +	{ 96000, TMDS_445_5M, 10240, 371250 },
>>> +	{ 192000, TMDS_445M, 23296, 421875 },
>>> +	{ 192000, TMDS_445_5M, 20480, 371250 },
>>> +};
>>> +
>>>   /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>>>   static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_state)
>>>   {
>>> @@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_sta
>>>   static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
>>>   				   int rate)
>>>   {
>>> -	const struct drm_display_mode *adjusted_mode =
>>> -		&crtc_state->base.adjusted_mode;
>>> -	int i;
>>> +	const struct hdmi_aud_ncts_table *hdmi_ncts_table;
>>> +	int i, size = 0;
>>> +
>>> +	if (crtc_state->pipe_bpp == 36) {
>>> +		hdmi_ncts_table = hdmi_aud_ncts_36bpp;
>>> +		size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
>>> +	} else if (crtc_state->pipe_bpp == 30) {
>>> +		hdmi_ncts_table = hdmi_aud_ncts_30bpp;
>>> +		size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
>>> +	} else {
>>> +		hdmi_ncts_table = hdmi_aud_ncts_24bpp;
>>> +		size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
>>> +	}
>>>   
>>> -	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
>>> -		if (rate == hdmi_aud_ncts[i].sample_rate &&
>>> -		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
>>> -			return hdmi_aud_ncts[i].n;
>>> +	for (i = 0;  i < size; i++) {
>>> +		if (rate == hdmi_ncts_table[i].sample_rate &&
>>> +		    crtc_state->port_clock == hdmi_ncts_table[i].clock) {
>>> +			return hdmi_ncts_table[i].n;
>>>   		}
>>>   	}
>>> +
>>>   	return 0;
>>>   }
>>>   
>>> -- 
>>> 2.17.1
>>>
>> Jani
>> Do you want me to change anything in this patch?
> Hey, please don't drop things on the floor if you don't hear from me!
>
> I didn't look at the spec, but it seems odd to me that the bpp would
> limit us to specific port clocks. Is that really so? What if you have,
> say, TMDS_594M and 36 bpp, this wouldn't find the params.

TMDS_594M and 36bpp is not possible. 594 x 1.5 (36 bit) would require a 
891MHz TMDS clock and the Max TMDS in the HDMI spec is 594 MHz. .

-Clint


>
> BR,
> Jani.
>
>
>> Regards,
>> Aditya Swarup
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color

[Jani Nikula]

On Tue, 02 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
> Adding N & CTS values for 10/12 bit deep color from Appendix C
> table in HDMI 2.0 spec. The correct values for N is not chosen
> automatically by hardware for deep color modes.
>
> v2: Remove redundant code and make it generic.(Jani)
>
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 82 +++++++++++++++++++++++++-----
>  1 file changed, 69 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 502b57ce72ab..ad53b04fa5a2 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -70,6 +70,13 @@ struct dp_aud_n_m {
>  	u16 n;
>  };
>  
> +struct hdmi_aud_ncts_table {

The struct itself is not a table. Just make it struct hdmi_ncts or
something.

> +	int sample_rate;
> +	int clock;
> +	int n;
> +	int cts;
> +};
> +
>  /* Values according to DP 1.4 Table 2-104 */
>  static const struct dp_aud_n_m dp_aud_n_m[] = {
>  	{ 32000, LC_162M, 1024, 10125 },
> @@ -146,12 +153,7 @@ static const struct {
>  #define TMDS_594M 594000
>  #define TMDS_593M 593407
>  
> -static const struct {
> -	int sample_rate;
> -	int clock;
> -	int n;
> -	int cts;
> -} hdmi_aud_ncts[] = {
> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
>  	{ 32000, TMDS_296M, 5824, 421875 },
>  	{ 32000, TMDS_297M, 3072, 222750 },
>  	{ 32000, TMDS_593M, 5824, 843750 },
> @@ -182,6 +184,49 @@ static const struct {
>  	{ 192000, TMDS_594M, 24576, 594000 },
>  };
>  
> +/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
> +/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
> +#define TMDS_371M 371250
> +#define TMDS_370M 370878
> +
> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
> +	{ 32000, TMDS_370M, 5824, 527344 },

N should be 11648?

> +	{ 32000, TMDS_371M, 6144, 556875 },
> +	{ 44100, TMDS_370M, 8918, 585938 },

N 17836

> +	{ 44100, TMDS_371M, 4704, 309375 },
> +	{ 88200, TMDS_370M, 17836, 585938 },

N 35672

> +	{ 88200, TMDS_371M, 9408, 309375 },
> +	{ 176400, TMDS_370M, 35672, 585938 },

N 71344

> +	{ 176400, TMDS_371M, 18816, 309375 },
> +	{ 48000, TMDS_370M, 11648, 703125 },
> +	{ 48000, TMDS_371M, 5120, 309375 },
> +	{ 96000, TMDS_370M, 23296, 703125 },
> +	{ 96000, TMDS_371M, 10240, 309375 },
> +	{ 192000, TMDS_370M, 46592, 703125 },
> +	{ 192000, TMDS_371M, 20480, 309375 },
> +};
> +
> +/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
> +#define TMDS_445_5M 445500
> +#define TMDS_445M 445054
> +
> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
> +	{ 32000, TMDS_445M, 5824, 632813 },
> +	{ 32000, TMDS_445_5M, 4096, 445500 },
> +	{ 44100, TMDS_445M, 8918, 703125 },
> +	{ 44100, TMDS_445_5M, 4704, 371250 },
> +	{ 88200, TMDS_445M, 17836, 703125 },
> +	{ 88200, TMDS_445_5M, 9408, 371250 },
> +	{ 176400, TMDS_445M, 35672, 703125 },
> +	{ 176400, TMDS_445_5M, 18816, 371250 },
> +	{ 48000, TMDS_445M, 5824, 421875 },
> +	{ 48000, TMDS_445_5M, 5120, 371250 },
> +	{ 96000, TMDS_445M, 11648, 421875 },
> +	{ 96000, TMDS_445_5M, 10240, 371250 },
> +	{ 192000, TMDS_445M, 23296, 421875 },
> +	{ 192000, TMDS_445_5M, 20480, 371250 },
> +};
> +
>  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>  static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_state)
>  {
> @@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_sta
>  static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
>  				   int rate)
>  {
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->base.adjusted_mode;
> -	int i;
> +	const struct hdmi_aud_ncts_table *hdmi_ncts_table;
> +	int i, size = 0;

Unnecessary initialization.

> +
> +	if (crtc_state->pipe_bpp == 36) {
> +		hdmi_ncts_table = hdmi_aud_ncts_36bpp;
> +		size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
> +	} else if (crtc_state->pipe_bpp == 30) {
> +		hdmi_ncts_table = hdmi_aud_ncts_30bpp;
> +		size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
> +	} else {
> +		hdmi_ncts_table = hdmi_aud_ncts_24bpp;
> +		size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
> +	}
>  
> -	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
> -		if (rate == hdmi_aud_ncts[i].sample_rate &&
> -		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
> -			return hdmi_aud_ncts[i].n;
> +	for (i = 0;  i < size; i++) {
                    ^
Superfluous space.


> +		if (rate == hdmi_ncts_table[i].sample_rate &&
> +		    crtc_state->port_clock == hdmi_ncts_table[i].clock) {

Changing from adjusted_mode->crtc_clock to crtc_state->port_clock is a
separate change that needs to be a separate patch.


BR,
Jani.

> +			return hdmi_ncts_table[i].n;
>  		}
>  	}
> +
>  	return 0;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color

[Aditya Swarup]

On Wed, May 15, 2019 at 05:40:10PM +0300, Jani Nikula wrote:
> On Tue, 02 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
> > Adding N & CTS values for 10/12 bit deep color from Appendix C
> > table in HDMI 2.0 spec. The correct values for N is not chosen
> > automatically by hardware for deep color modes.
> >
> > v2: Remove redundant code and make it generic.(Jani)
> >
> > Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_audio.c | 82 +++++++++++++++++++++++++-----
> >  1 file changed, 69 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > index 502b57ce72ab..ad53b04fa5a2 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -70,6 +70,13 @@ struct dp_aud_n_m {
> >  	u16 n;
> >  };
> >  
> > +struct hdmi_aud_ncts_table {
> 
> The struct itself is not a table. Just make it struct hdmi_ncts or
> something.
Will do it in the next spin.
> 
> > +	int sample_rate;
> > +	int clock;
> > +	int n;
> > +	int cts;
> > +};
> > +
> >  /* Values according to DP 1.4 Table 2-104 */
> >  static const struct dp_aud_n_m dp_aud_n_m[] = {
> >  	{ 32000, LC_162M, 1024, 10125 },
> > @@ -146,12 +153,7 @@ static const struct {
> >  #define TMDS_594M 594000
> >  #define TMDS_593M 593407
> >  
> > -static const struct {
> > -	int sample_rate;
> > -	int clock;
> > -	int n;
> > -	int cts;
> > -} hdmi_aud_ncts[] = {
> > +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
> >  	{ 32000, TMDS_296M, 5824, 421875 },
> >  	{ 32000, TMDS_297M, 3072, 222750 },
> >  	{ 32000, TMDS_593M, 5824, 843750 },
> > @@ -182,6 +184,49 @@ static const struct {
> >  	{ 192000, TMDS_594M, 24576, 594000 },
> >  };
> >  
> > +/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
> > +/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
> > +#define TMDS_371M 371250
> > +#define TMDS_370M 370878
> > +
> > +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
> > +	{ 32000, TMDS_370M, 5824, 527344 },
> 
> N should be 11648?
The value in the struct is correct. It is 5824 - Appendix C Table C-1 Pg
234, for 371.25/1.001 and f = 32kHz => N = 5824
> 
> > +	{ 32000, TMDS_371M, 6144, 556875 },
> > +	{ 44100, TMDS_370M, 8918, 585938 },
> 
> N 17836
Value in the table is correct.
> 
> > +	{ 44100, TMDS_371M, 4704, 309375 },
> > +	{ 88200, TMDS_370M, 17836, 585938 },
> 
> N 35672
Value in the table is correct.
> 
> > +	{ 88200, TMDS_371M, 9408, 309375 },
> > +	{ 176400, TMDS_370M, 35672, 585938 },
> 
> N 71344
Value in the table is correct.
> 
> > +	{ 176400, TMDS_371M, 18816, 309375 },
> > +	{ 48000, TMDS_370M, 11648, 703125 },
> > +	{ 48000, TMDS_371M, 5120, 309375 },
> > +	{ 96000, TMDS_370M, 23296, 703125 },
> > +	{ 96000, TMDS_371M, 10240, 309375 },
> > +	{ 192000, TMDS_370M, 46592, 703125 },
> > +	{ 192000, TMDS_371M, 20480, 309375 },
> > +};
> > +
> > +/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
> > +#define TMDS_445_5M 445500
> > +#define TMDS_445M 445054
> > +
> > +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
> > +	{ 32000, TMDS_445M, 5824, 632813 },
> > +	{ 32000, TMDS_445_5M, 4096, 445500 },
> > +	{ 44100, TMDS_445M, 8918, 703125 },
> > +	{ 44100, TMDS_445_5M, 4704, 371250 },
> > +	{ 88200, TMDS_445M, 17836, 703125 },
> > +	{ 88200, TMDS_445_5M, 9408, 371250 },
> > +	{ 176400, TMDS_445M, 35672, 703125 },
> > +	{ 176400, TMDS_445_5M, 18816, 371250 },
> > +	{ 48000, TMDS_445M, 5824, 421875 },
> > +	{ 48000, TMDS_445_5M, 5120, 371250 },
> > +	{ 96000, TMDS_445M, 11648, 421875 },
> > +	{ 96000, TMDS_445_5M, 10240, 371250 },
> > +	{ 192000, TMDS_445M, 23296, 421875 },
> > +	{ 192000, TMDS_445_5M, 20480, 371250 },
> > +};
> > +
> >  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
> >  static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_state)
> >  {
> > @@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_sta
> >  static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
> >  				   int rate)
> >  {
> > -	const struct drm_display_mode *adjusted_mode =
> > -		&crtc_state->base.adjusted_mode;
> > -	int i;
> > +	const struct hdmi_aud_ncts_table *hdmi_ncts_table;
> > +	int i, size = 0;
> 
> Unnecessary initialization.
> 
> > +
> > +	if (crtc_state->pipe_bpp == 36) {
> > +		hdmi_ncts_table = hdmi_aud_ncts_36bpp;
> > +		size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
> > +	} else if (crtc_state->pipe_bpp == 30) {
> > +		hdmi_ncts_table = hdmi_aud_ncts_30bpp;
> > +		size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
> > +	} else {
> > +		hdmi_ncts_table = hdmi_aud_ncts_24bpp;
> > +		size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
> > +	}
> >  
> > -	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
> > -		if (rate == hdmi_aud_ncts[i].sample_rate &&
> > -		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
> > -			return hdmi_aud_ncts[i].n;
> > +	for (i = 0;  i < size; i++) {
>                     ^
> Superfluous space.
Will correct it in next spin.
> 
> 
> > +		if (rate == hdmi_ncts_table[i].sample_rate &&
> > +		    crtc_state->port_clock == hdmi_ncts_table[i].clock) {
> 
> Changing from adjusted_mode->crtc_clock to crtc_state->port_clock is a
> separate change that needs to be a separate patch.
I don't think it should be a separate change. Working with deep color
mode requires crtc_state->port_clock as it is scaled according to the
deep color mode set. Using adjusted_mode->crtc_clock is incorrect as it
doesn't account for deep color mode; we will have to multiply with
scaling factor for the set deep color mode which is superfluous.

I can add a comment in the commit message and comment before the if
block to explain the change. Let me know if it works?
> 
> 
> BR,
> Jani.
> 
> > +			return hdmi_ncts_table[i].n;
> >  		}
> >  	}
> > +
> >  	return 0;
> >  }
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

Regards,
Aditya Swarup
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color

[Jani Nikula]

On Fri, 17 May 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
> On Wed, May 15, 2019 at 05:40:10PM +0300, Jani Nikula wrote:
>> On Tue, 02 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
>> > Adding N & CTS values for 10/12 bit deep color from Appendix C
>> > table in HDMI 2.0 spec. The correct values for N is not chosen
>> > automatically by hardware for deep color modes.
>> >
>> > v2: Remove redundant code and make it generic.(Jani)
>> >
>> > Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>> > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Cc: Jani Nikula <jani.nikula@intel.com>
>> > Cc: Manasi Navare <manasi.d.navare@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_audio.c | 82 +++++++++++++++++++++++++-----
>> >  1 file changed, 69 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> > index 502b57ce72ab..ad53b04fa5a2 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -70,6 +70,13 @@ struct dp_aud_n_m {
>> >  	u16 n;
>> >  };
>> >  
>> > +struct hdmi_aud_ncts_table {
>> 
>> The struct itself is not a table. Just make it struct hdmi_ncts or
>> something.
> Will do it in the next spin.
>> 
>> > +	int sample_rate;
>> > +	int clock;
>> > +	int n;
>> > +	int cts;
>> > +};
>> > +
>> >  /* Values according to DP 1.4 Table 2-104 */
>> >  static const struct dp_aud_n_m dp_aud_n_m[] = {
>> >  	{ 32000, LC_162M, 1024, 10125 },
>> > @@ -146,12 +153,7 @@ static const struct {
>> >  #define TMDS_594M 594000
>> >  #define TMDS_593M 593407
>> >  
>> > -static const struct {
>> > -	int sample_rate;
>> > -	int clock;
>> > -	int n;
>> > -	int cts;
>> > -} hdmi_aud_ncts[] = {
>> > +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
>> >  	{ 32000, TMDS_296M, 5824, 421875 },
>> >  	{ 32000, TMDS_297M, 3072, 222750 },
>> >  	{ 32000, TMDS_593M, 5824, 843750 },
>> > @@ -182,6 +184,49 @@ static const struct {
>> >  	{ 192000, TMDS_594M, 24576, 594000 },
>> >  };
>> >  
>> > +/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
>> > +/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
>> > +#define TMDS_371M 371250
>> > +#define TMDS_370M 370878
>> > +
>> > +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
>> > +	{ 32000, TMDS_370M, 5824, 527344 },
>> 
>> N should be 11648?
> The value in the struct is correct. It is 5824 - Appendix C Table C-1 Pg
> 234, for 371.25/1.001 and f = 32kHz => N = 5824

Huh, looks like I have an outdated version of the spec then. For me the
table is on page 232.

Need to find the up-to-date spec, but looking at draft 2.1 the values
you have match.

>> 
>> > +	{ 32000, TMDS_371M, 6144, 556875 },
>> > +	{ 44100, TMDS_370M, 8918, 585938 },
>> 
>> N 17836
> Value in the table is correct.
>> 
>> > +	{ 44100, TMDS_371M, 4704, 309375 },
>> > +	{ 88200, TMDS_370M, 17836, 585938 },
>> 
>> N 35672
> Value in the table is correct.
>> 
>> > +	{ 88200, TMDS_371M, 9408, 309375 },
>> > +	{ 176400, TMDS_370M, 35672, 585938 },
>> 
>> N 71344
> Value in the table is correct.
>> 
>> > +	{ 176400, TMDS_371M, 18816, 309375 },
>> > +	{ 48000, TMDS_370M, 11648, 703125 },
>> > +	{ 48000, TMDS_371M, 5120, 309375 },
>> > +	{ 96000, TMDS_370M, 23296, 703125 },
>> > +	{ 96000, TMDS_371M, 10240, 309375 },
>> > +	{ 192000, TMDS_370M, 46592, 703125 },
>> > +	{ 192000, TMDS_371M, 20480, 309375 },
>> > +};
>> > +
>> > +/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
>> > +#define TMDS_445_5M 445500
>> > +#define TMDS_445M 445054
>> > +
>> > +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
>> > +	{ 32000, TMDS_445M, 5824, 632813 },
>> > +	{ 32000, TMDS_445_5M, 4096, 445500 },
>> > +	{ 44100, TMDS_445M, 8918, 703125 },
>> > +	{ 44100, TMDS_445_5M, 4704, 371250 },
>> > +	{ 88200, TMDS_445M, 17836, 703125 },
>> > +	{ 88200, TMDS_445_5M, 9408, 371250 },
>> > +	{ 176400, TMDS_445M, 35672, 703125 },
>> > +	{ 176400, TMDS_445_5M, 18816, 371250 },
>> > +	{ 48000, TMDS_445M, 5824, 421875 },
>> > +	{ 48000, TMDS_445_5M, 5120, 371250 },
>> > +	{ 96000, TMDS_445M, 11648, 421875 },
>> > +	{ 96000, TMDS_445_5M, 10240, 371250 },
>> > +	{ 192000, TMDS_445M, 23296, 421875 },
>> > +	{ 192000, TMDS_445_5M, 20480, 371250 },
>> > +};
>> > +
>> >  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>> >  static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_state)
>> >  {
>> > @@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_sta
>> >  static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
>> >  				   int rate)
>> >  {
>> > -	const struct drm_display_mode *adjusted_mode =
>> > -		&crtc_state->base.adjusted_mode;
>> > -	int i;
>> > +	const struct hdmi_aud_ncts_table *hdmi_ncts_table;
>> > +	int i, size = 0;
>> 
>> Unnecessary initialization.
>> 
>> > +
>> > +	if (crtc_state->pipe_bpp == 36) {
>> > +		hdmi_ncts_table = hdmi_aud_ncts_36bpp;
>> > +		size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
>> > +	} else if (crtc_state->pipe_bpp == 30) {
>> > +		hdmi_ncts_table = hdmi_aud_ncts_30bpp;
>> > +		size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
>> > +	} else {
>> > +		hdmi_ncts_table = hdmi_aud_ncts_24bpp;
>> > +		size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
>> > +	}
>> >  
>> > -	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
>> > -		if (rate == hdmi_aud_ncts[i].sample_rate &&
>> > -		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
>> > -			return hdmi_aud_ncts[i].n;
>> > +	for (i = 0;  i < size; i++) {
>>                     ^
>> Superfluous space.
> Will correct it in next spin.
>> 
>> 
>> > +		if (rate == hdmi_ncts_table[i].sample_rate &&
>> > +		    crtc_state->port_clock == hdmi_ncts_table[i].clock) {
>> 
>> Changing from adjusted_mode->crtc_clock to crtc_state->port_clock is a
>> separate change that needs to be a separate patch.
> I don't think it should be a separate change. Working with deep color
> mode requires crtc_state->port_clock as it is scaled according to the
> deep color mode set. Using adjusted_mode->crtc_clock is incorrect as it
> doesn't account for deep color mode; we will have to multiply with
> scaling factor for the set deep color mode which is superfluous.

Make the crtc_clock -> port_clock change *before* the deep color
changes, that's the whole point. Ensure that this change does not impact
non-deep color, and if it does, the bisect will point at the minimal
change. Then add the deep color N & CTS when the code already uses
port_clock.

BR,
Jani.

>
> I can add a comment in the commit message and comment before the if
> block to explain the change. Let me know if it works?
>> 
>> 
>> BR,
>> Jani.
>> 
>> > +			return hdmi_ncts_table[i].n;
>> >  		}
>> >  	}
>> > +
>> >  	return 0;
>> >  }
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center
>
> Regards,
> Aditya Swarup

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color

[Jani Nikula]

On Fri, 17 May 2019, Jani Nikula <jani.nikula@intel.com> wrote:
> On Fri, 17 May 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
>> On Wed, May 15, 2019 at 05:40:10PM +0300, Jani Nikula wrote:
>>> On Tue, 02 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
>>> > Adding N & CTS values for 10/12 bit deep color from Appendix C
>>> > table in HDMI 2.0 spec. The correct values for N is not chosen
>>> > automatically by hardware for deep color modes.
>>> >
>>> > v2: Remove redundant code and make it generic.(Jani)
>>> >
>>> > Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>>> > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
>>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> > Cc: Jani Nikula <jani.nikula@intel.com>
>>> > Cc: Manasi Navare <manasi.d.navare@intel.com>
>>> > ---
>>> >  drivers/gpu/drm/i915/intel_audio.c | 82 +++++++++++++++++++++++++-----
>>> >  1 file changed, 69 insertions(+), 13 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>> > index 502b57ce72ab..ad53b04fa5a2 100644
>>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>>> > @@ -70,6 +70,13 @@ struct dp_aud_n_m {
>>> >  	u16 n;
>>> >  };
>>> >  
>>> > +struct hdmi_aud_ncts_table {
>>> 
>>> The struct itself is not a table. Just make it struct hdmi_ncts or
>>> something.
>> Will do it in the next spin.

Aditya, please provide the updated version.

BR,
Jani.

>>> 
>>> > +	int sample_rate;
>>> > +	int clock;
>>> > +	int n;
>>> > +	int cts;
>>> > +};
>>> > +
>>> >  /* Values according to DP 1.4 Table 2-104 */
>>> >  static const struct dp_aud_n_m dp_aud_n_m[] = {
>>> >  	{ 32000, LC_162M, 1024, 10125 },
>>> > @@ -146,12 +153,7 @@ static const struct {
>>> >  #define TMDS_594M 594000
>>> >  #define TMDS_593M 593407
>>> >  
>>> > -static const struct {
>>> > -	int sample_rate;
>>> > -	int clock;
>>> > -	int n;
>>> > -	int cts;
>>> > -} hdmi_aud_ncts[] = {
>>> > +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
>>> >  	{ 32000, TMDS_296M, 5824, 421875 },
>>> >  	{ 32000, TMDS_297M, 3072, 222750 },
>>> >  	{ 32000, TMDS_593M, 5824, 843750 },
>>> > @@ -182,6 +184,49 @@ static const struct {
>>> >  	{ 192000, TMDS_594M, 24576, 594000 },
>>> >  };
>>> >  
>>> > +/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
>>> > +/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
>>> > +#define TMDS_371M 371250
>>> > +#define TMDS_370M 370878
>>> > +
>>> > +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
>>> > +	{ 32000, TMDS_370M, 5824, 527344 },
>>> 
>>> N should be 11648?
>> The value in the struct is correct. It is 5824 - Appendix C Table C-1 Pg
>> 234, for 371.25/1.001 and f = 32kHz => N = 5824
>
> Huh, looks like I have an outdated version of the spec then. For me the
> table is on page 232.
>
> Need to find the up-to-date spec, but looking at draft 2.1 the values
> you have match.
>
>>> 
>>> > +	{ 32000, TMDS_371M, 6144, 556875 },
>>> > +	{ 44100, TMDS_370M, 8918, 585938 },
>>> 
>>> N 17836
>> Value in the table is correct.
>>> 
>>> > +	{ 44100, TMDS_371M, 4704, 309375 },
>>> > +	{ 88200, TMDS_370M, 17836, 585938 },
>>> 
>>> N 35672
>> Value in the table is correct.
>>> 
>>> > +	{ 88200, TMDS_371M, 9408, 309375 },
>>> > +	{ 176400, TMDS_370M, 35672, 585938 },
>>> 
>>> N 71344
>> Value in the table is correct.
>>> 
>>> > +	{ 176400, TMDS_371M, 18816, 309375 },
>>> > +	{ 48000, TMDS_370M, 11648, 703125 },
>>> > +	{ 48000, TMDS_371M, 5120, 309375 },
>>> > +	{ 96000, TMDS_370M, 23296, 703125 },
>>> > +	{ 96000, TMDS_371M, 10240, 309375 },
>>> > +	{ 192000, TMDS_370M, 46592, 703125 },
>>> > +	{ 192000, TMDS_371M, 20480, 309375 },
>>> > +};
>>> > +
>>> > +/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
>>> > +#define TMDS_445_5M 445500
>>> > +#define TMDS_445M 445054
>>> > +
>>> > +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
>>> > +	{ 32000, TMDS_445M, 5824, 632813 },
>>> > +	{ 32000, TMDS_445_5M, 4096, 445500 },
>>> > +	{ 44100, TMDS_445M, 8918, 703125 },
>>> > +	{ 44100, TMDS_445_5M, 4704, 371250 },
>>> > +	{ 88200, TMDS_445M, 17836, 703125 },
>>> > +	{ 88200, TMDS_445_5M, 9408, 371250 },
>>> > +	{ 176400, TMDS_445M, 35672, 703125 },
>>> > +	{ 176400, TMDS_445_5M, 18816, 371250 },
>>> > +	{ 48000, TMDS_445M, 5824, 421875 },
>>> > +	{ 48000, TMDS_445_5M, 5120, 371250 },
>>> > +	{ 96000, TMDS_445M, 11648, 421875 },
>>> > +	{ 96000, TMDS_445_5M, 10240, 371250 },
>>> > +	{ 192000, TMDS_445M, 23296, 421875 },
>>> > +	{ 192000, TMDS_445_5M, 20480, 371250 },
>>> > +};
>>> > +
>>> >  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>>> >  static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_state)
>>> >  {
>>> > @@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_sta
>>> >  static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
>>> >  				   int rate)
>>> >  {
>>> > -	const struct drm_display_mode *adjusted_mode =
>>> > -		&crtc_state->base.adjusted_mode;
>>> > -	int i;
>>> > +	const struct hdmi_aud_ncts_table *hdmi_ncts_table;
>>> > +	int i, size = 0;
>>> 
>>> Unnecessary initialization.
>>> 
>>> > +
>>> > +	if (crtc_state->pipe_bpp == 36) {
>>> > +		hdmi_ncts_table = hdmi_aud_ncts_36bpp;
>>> > +		size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
>>> > +	} else if (crtc_state->pipe_bpp == 30) {
>>> > +		hdmi_ncts_table = hdmi_aud_ncts_30bpp;
>>> > +		size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
>>> > +	} else {
>>> > +		hdmi_ncts_table = hdmi_aud_ncts_24bpp;
>>> > +		size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
>>> > +	}
>>> >  
>>> > -	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
>>> > -		if (rate == hdmi_aud_ncts[i].sample_rate &&
>>> > -		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
>>> > -			return hdmi_aud_ncts[i].n;
>>> > +	for (i = 0;  i < size; i++) {
>>>                     ^
>>> Superfluous space.
>> Will correct it in next spin.
>>> 
>>> 
>>> > +		if (rate == hdmi_ncts_table[i].sample_rate &&
>>> > +		    crtc_state->port_clock == hdmi_ncts_table[i].clock) {
>>> 
>>> Changing from adjusted_mode->crtc_clock to crtc_state->port_clock is a
>>> separate change that needs to be a separate patch.
>> I don't think it should be a separate change. Working with deep color
>> mode requires crtc_state->port_clock as it is scaled according to the
>> deep color mode set. Using adjusted_mode->crtc_clock is incorrect as it
>> doesn't account for deep color mode; we will have to multiply with
>> scaling factor for the set deep color mode which is superfluous.
>
> Make the crtc_clock -> port_clock change *before* the deep color
> changes, that's the whole point. Ensure that this change does not impact
> non-deep color, and if it does, the bisect will point at the minimal
> change. Then add the deep color N & CTS when the code already uses
> port_clock.
>
> BR,
> Jani.
>
>>
>> I can add a comment in the commit message and comment before the if
>> block to explain the change. Let me know if it works?
>>> 
>>> 
>>> BR,
>>> Jani.
>>> 
>>> > +			return hdmi_ncts_table[i].n;
>>> >  		}
>>> >  	}
>>> > +
>>> >  	return 0;
>>> >  }
>>> 
>>> -- 
>>> Jani Nikula, Intel Open Source Graphics Center
>>
>> Regards,
>> Aditya Swarup

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

[Ville Syrjälä]

On Tue, Apr 02, 2019 at 05:14:39AM -0700, Aditya Swarup wrote:
> From: Clinton Taylor <Clinton.A.Taylor@intel.com>
> 
> v2: Fix commit msg to reflect why issue occurs(Jani)
> Set GCP_COLOR_INDICATION only when we set 10/12 bit deep color.
> 
> Changing settings from 10/12 bit deep color to 8 bit(& vice versa)
> doesn't work correctly using xrandr max bpc property. When we
> connect a monitor which supports deep color, the highest deep color
> setting is selected; which sets GCP_COLOR_INDICATION. When we change
> the setting to 8 bit color, we still set GCP_COLOR_INDICATION which
> doesn't allow the switch back to 8 bit color.

Why not? Table 6-1 in HDMI 1.4a spec clearly lists 8bpc as a
valid setting for GCP.

> 
> Signed-off-by: Clinton Taylor <Clinton.A.Taylor@intel.com>
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 5ccb305a6e1c..4760462f84ca 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -963,7 +963,9 @@ static void intel_hdmi_compute_gcp_infoframe(struct intel_encoder *encoder,
>  		intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GENERAL_CONTROL);
>  
>  	/* Indicate color depth whenever the sink supports deep color */
> -	if (hdmi_sink_is_deep_color(conn_state))
> +
> +	if (hdmi_sink_is_deep_color(conn_state) &&
> +	    (crtc_state->pipe_bpp > 24))
>  		crtc_state->infoframes.gcp |= GCP_COLOR_INDICATION;
>  
>  	/* Enable default_phase whenever the display mode is suitably aligned */
> @@ -2260,6 +2262,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
>  	int clock_10bpc = clock_8bpc * 5 / 4;
>  	int clock_12bpc = clock_8bpc * 3 / 2;
> +	int dc_clock = clock_12bpc;
>  	int desired_bpp;
>  	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
>  
> @@ -2314,22 +2317,18 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	 * Note that g4x/vlv don't support 12bpc hdmi outputs. We also need
>  	 * to check that the higher clock still fits within limits.
>  	 */
> -	if (hdmi_deep_color_possible(pipe_config, 12) &&
> -	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc,
> +	if (pipe_config->pipe_bpp == 30)
> +		dc_clock = clock_10bpc;
> +
> +	if (hdmi_deep_color_possible(pipe_config, pipe_config->pipe_bpp / 3) &&
> +	    hdmi_port_clock_valid(intel_hdmi, dc_clock,
>  				  true, force_dvi) == MODE_OK) {
> -		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
> -		desired_bpp = 12*3;
> -
> -		/* Need to adjust the port link by 1.5x for 12bpc. */
> -		pipe_config->port_clock = clock_12bpc;
> -	} else if (hdmi_deep_color_possible(pipe_config, 10) &&
> -		   hdmi_port_clock_valid(intel_hdmi, clock_10bpc,
> -					 true, force_dvi) == MODE_OK) {
> -		DRM_DEBUG_KMS("picking bpc to 10 for HDMI output\n");
> -		desired_bpp = 10 * 3;
> -
> -		/* Need to adjust the port link by 1.25x for 10bpc. */
> -		pipe_config->port_clock = clock_10bpc;
> +		DRM_DEBUG_KMS("picking bpc to %d for HDMI output\n",
> +				 pipe_config->pipe_bpp / 3);
> +		desired_bpp = pipe_config->pipe_bpp;
> +
> +		/* Need to adjust the port link dc modes. */
> +		pipe_config->port_clock = dc_clock;

This part has nothing to do with the commit message.


>  	} else {
>  		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
>  		desired_bpp = 8*3;
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

[Clinton Taylor]

On 4/2/19 5:53 AM, Ville Syrjälä wrote:
> On Tue, Apr 02, 2019 at 05:14:39AM -0700, Aditya Swarup wrote:
>> From: Clinton Taylor <Clinton.A.Taylor@intel.com>
>>
>> v2: Fix commit msg to reflect why issue occurs(Jani)
>> Set GCP_COLOR_INDICATION only when we set 10/12 bit deep color.
>>
>> Changing settings from 10/12 bit deep color to 8 bit(& vice versa)
>> doesn't work correctly using xrandr max bpc property. When we
>> connect a monitor which supports deep color, the highest deep color
>> setting is selected; which sets GCP_COLOR_INDICATION. When we change
>> the setting to 8 bit color, we still set GCP_COLOR_INDICATION which
>> doesn't allow the switch back to 8 bit color.
> Why not? Table 6-1 in HDMI 1.4a spec clearly lists 8bpc as a
> valid setting for GCP.

This appears to be a undocumented SI issue. According to the spec 8 bit 
is a valid GCP value. However, if you enable GCP indication and 8 bit is 
current then 10 bit is transmit via GCP confusing monitors. The GCP is 
being validated using 2 different HDMI Analyzer's confirming the results.

We will file a SI bug against the current behavior.

-Clint


>
>> Signed-off-by: Clinton Taylor <Clinton.A.Taylor@intel.com>
>> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_hdmi.c | 31 +++++++++++++++----------------
>>   1 file changed, 15 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 5ccb305a6e1c..4760462f84ca 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -963,7 +963,9 @@ static void intel_hdmi_compute_gcp_infoframe(struct intel_encoder *encoder,
>>   		intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GENERAL_CONTROL);
>>   
>>   	/* Indicate color depth whenever the sink supports deep color */
>> -	if (hdmi_sink_is_deep_color(conn_state))
>> +
>> +	if (hdmi_sink_is_deep_color(conn_state) &&
>> +	    (crtc_state->pipe_bpp > 24))
>>   		crtc_state->infoframes.gcp |= GCP_COLOR_INDICATION;
>>   
>>   	/* Enable default_phase whenever the display mode is suitably aligned */
>> @@ -2260,6 +2262,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
>>   	int clock_10bpc = clock_8bpc * 5 / 4;
>>   	int clock_12bpc = clock_8bpc * 3 / 2;
>> +	int dc_clock = clock_12bpc;
>>   	int desired_bpp;
>>   	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
>>   
>> @@ -2314,22 +2317,18 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   	 * Note that g4x/vlv don't support 12bpc hdmi outputs. We also need
>>   	 * to check that the higher clock still fits within limits.
>>   	 */
>> -	if (hdmi_deep_color_possible(pipe_config, 12) &&
>> -	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc,
>> +	if (pipe_config->pipe_bpp == 30)
>> +		dc_clock = clock_10bpc;
>> +
>> +	if (hdmi_deep_color_possible(pipe_config, pipe_config->pipe_bpp / 3) &&
>> +	    hdmi_port_clock_valid(intel_hdmi, dc_clock,
>>   				  true, force_dvi) == MODE_OK) {
>> -		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
>> -		desired_bpp = 12*3;
>> -
>> -		/* Need to adjust the port link by 1.5x for 12bpc. */
>> -		pipe_config->port_clock = clock_12bpc;
>> -	} else if (hdmi_deep_color_possible(pipe_config, 10) &&
>> -		   hdmi_port_clock_valid(intel_hdmi, clock_10bpc,
>> -					 true, force_dvi) == MODE_OK) {
>> -		DRM_DEBUG_KMS("picking bpc to 10 for HDMI output\n");
>> -		desired_bpp = 10 * 3;
>> -
>> -		/* Need to adjust the port link by 1.25x for 10bpc. */
>> -		pipe_config->port_clock = clock_10bpc;
>> +		DRM_DEBUG_KMS("picking bpc to %d for HDMI output\n",
>> +				 pipe_config->pipe_bpp / 3);
>> +		desired_bpp = pipe_config->pipe_bpp;
>> +
>> +		/* Need to adjust the port link dc modes. */
>> +		pipe_config->port_clock = dc_clock;
> This part has nothing to do with the commit message.
>
>
>>   	} else {
>>   		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
>>   		desired_bpp = 8*3;
>> -- 
>> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

[Clinton Taylor]

On 4/2/19 8:54 AM, Clinton Taylor wrote:
>
> On 4/2/19 5:53 AM, Ville Syrjälä wrote:
>> On Tue, Apr 02, 2019 at 05:14:39AM -0700, Aditya Swarup wrote:
>>> From: Clinton Taylor <Clinton.A.Taylor@intel.com>
>>>
>>> v2: Fix commit msg to reflect why issue occurs(Jani)
>>> Set GCP_COLOR_INDICATION only when we set 10/12 bit deep color.
>>>
>>> Changing settings from 10/12 bit deep color to 8 bit(& vice versa)
>>> doesn't work correctly using xrandr max bpc property. When we
>>> connect a monitor which supports deep color, the highest deep color
>>> setting is selected; which sets GCP_COLOR_INDICATION. When we change
>>> the setting to 8 bit color, we still set GCP_COLOR_INDICATION which
>>> doesn't allow the switch back to 8 bit color.
>> Why not? Table 6-1 in HDMI 1.4a spec clearly lists 8bpc as a
>> valid setting for GCP.
>
> This appears to be a undocumented SI issue. According to the spec 8 
> bit is a valid GCP value. However, if you enable GCP indication and 8 
> bit is current then 10 bit is transmit via GCP confusing monitors. The 
> GCP is being validated using 2 different HDMI Analyzer's confirming 
> the results.

After double checking BSPEC (50524) this behavior appears to be as as 
designed. Under Restriction for bit 2: This bit must be set when in HDMI 
deep color (>8 BPC) mode. I don't agree with this restriction as 8 bit 
is a valid GCP indication.

-Clint


>
> We will file a SI bug against the current behavior.
>
> -Clint
>
>
>>
>>> Signed-off-by: Clinton Taylor <Clinton.A.Taylor@intel.com>
>>> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_hdmi.c | 31 
>>> +++++++++++++++----------------
>>>   1 file changed, 15 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
>>> b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index 5ccb305a6e1c..4760462f84ca 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -963,7 +963,9 @@ static void 
>>> intel_hdmi_compute_gcp_infoframe(struct intel_encoder *encoder,
>>> intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GENERAL_CONTROL);
>>>         /* Indicate color depth whenever the sink supports deep 
>>> color */
>>> -    if (hdmi_sink_is_deep_color(conn_state))
>>> +
>>> +    if (hdmi_sink_is_deep_color(conn_state) &&
>>> +        (crtc_state->pipe_bpp > 24))
>>>           crtc_state->infoframes.gcp |= GCP_COLOR_INDICATION;
>>>         /* Enable default_phase whenever the display mode is 
>>> suitably aligned */
>>> @@ -2260,6 +2262,7 @@ int intel_hdmi_compute_config(struct 
>>> intel_encoder *encoder,
>>>       int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
>>>       int clock_10bpc = clock_8bpc * 5 / 4;
>>>       int clock_12bpc = clock_8bpc * 3 / 2;
>>> +    int dc_clock = clock_12bpc;
>>>       int desired_bpp;
>>>       bool force_dvi = intel_conn_state->force_audio == 
>>> HDMI_AUDIO_OFF_DVI;
>>>   @@ -2314,22 +2317,18 @@ int intel_hdmi_compute_config(struct 
>>> intel_encoder *encoder,
>>>        * Note that g4x/vlv don't support 12bpc hdmi outputs. We also 
>>> need
>>>        * to check that the higher clock still fits within limits.
>>>        */
>>> -    if (hdmi_deep_color_possible(pipe_config, 12) &&
>>> -        hdmi_port_clock_valid(intel_hdmi, clock_12bpc,
>>> +    if (pipe_config->pipe_bpp == 30)
>>> +        dc_clock = clock_10bpc;
>>> +
>>> +    if (hdmi_deep_color_possible(pipe_config, pipe_config->pipe_bpp 
>>> / 3) &&
>>> +        hdmi_port_clock_valid(intel_hdmi, dc_clock,
>>>                     true, force_dvi) == MODE_OK) {
>>> -        DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
>>> -        desired_bpp = 12*3;
>>> -
>>> -        /* Need to adjust the port link by 1.5x for 12bpc. */
>>> -        pipe_config->port_clock = clock_12bpc;
>>> -    } else if (hdmi_deep_color_possible(pipe_config, 10) &&
>>> -           hdmi_port_clock_valid(intel_hdmi, clock_10bpc,
>>> -                     true, force_dvi) == MODE_OK) {
>>> -        DRM_DEBUG_KMS("picking bpc to 10 for HDMI output\n");
>>> -        desired_bpp = 10 * 3;
>>> -
>>> -        /* Need to adjust the port link by 1.25x for 10bpc. */
>>> -        pipe_config->port_clock = clock_10bpc;
>>> +        DRM_DEBUG_KMS("picking bpc to %d for HDMI output\n",
>>> +                 pipe_config->pipe_bpp / 3);
>>> +        desired_bpp = pipe_config->pipe_bpp;
>>> +
>>> +        /* Need to adjust the port link dc modes. */
>>> +        pipe_config->port_clock = dc_clock;
>> This part has nothing to do with the commit message.
>>
>>
>>>       } else {
>>>           DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
>>>           desired_bpp = 8*3;
>>> -- 
>>> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

[Ville Syrjälä]

On Tue, Apr 02, 2019 at 09:25:58AM -0700, Clinton Taylor wrote:
> 
> On 4/2/19 8:54 AM, Clinton Taylor wrote:
> >
> > On 4/2/19 5:53 AM, Ville Syrjälä wrote:
> >> On Tue, Apr 02, 2019 at 05:14:39AM -0700, Aditya Swarup wrote:
> >>> From: Clinton Taylor <Clinton.A.Taylor@intel.com>
> >>>
> >>> v2: Fix commit msg to reflect why issue occurs(Jani)
> >>> Set GCP_COLOR_INDICATION only when we set 10/12 bit deep color.
> >>>
> >>> Changing settings from 10/12 bit deep color to 8 bit(& vice versa)
> >>> doesn't work correctly using xrandr max bpc property. When we
> >>> connect a monitor which supports deep color, the highest deep color
> >>> setting is selected; which sets GCP_COLOR_INDICATION. When we change
> >>> the setting to 8 bit color, we still set GCP_COLOR_INDICATION which
> >>> doesn't allow the switch back to 8 bit color.
> >> Why not? Table 6-1 in HDMI 1.4a spec clearly lists 8bpc as a
> >> valid setting for GCP.
> >
> > This appears to be a undocumented SI issue. According to the spec 8 
> > bit is a valid GCP value. However, if you enable GCP indication and 8 
> > bit is current then 10 bit is transmit via GCP confusing monitors. The 
> > GCP is being validated using 2 different HDMI Analyzer's confirming 
> > the results.
> 
> After double checking BSPEC (50524) this behavior appears to be as as 
> designed. Under Restriction for bit 2: This bit must be set when in HDMI 
> deep color (>8 BPC) mode. I don't agree with this restriction as 8 bit 
> is a valid GCP indication.

That doesn't say we musn't enable it with 8bpc.

However current Bspec has this to say:
"GCP is only supported with HDMI when the bits per color is not equal
 to 8. GCP must be enabled prior to enabling TRANS_DDI_FUNC_CTL for HDMI
 with bits per color not equal to 8 and disabled after disabling
 TRANS_DDI_FUNC_CTL"

So yeah, looks like this isn't supported by our current hw.

It was supported in the past. Eg. CPT bspec still has this to say:
"This bit must be set when in deep color mode. It may optionally be set
 for 24-bit mode. It must be set if the sink attached to the transcoder
 can receive GCP data."

But HSW bspec already forbids it. So looks like we have to disable this 
on HSW+ at least. Probably easier to just disable it across the board.
The spec seems to suggest that we should disable GCP entirely in 8bpc
mode, but there are some changlog entries in the HSW docs that
suggest to me that setting CD=0 should be sufficient. I guess that
agrees with your patch.

> 
> -Clint
> 
> 
> >
> > We will file a SI bug against the current behavior.
> >
> > -Clint
> >
> >
> >>
> >>> Signed-off-by: Clinton Taylor <Clinton.A.Taylor@intel.com>
> >>> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> Cc: Jani Nikula <jani.nikula@intel.com>
> >>> Cc: Manasi Navare <manasi.d.navare@intel.com>
> >>> ---
> >>>   drivers/gpu/drm/i915/intel_hdmi.c | 31 
> >>> +++++++++++++++----------------
> >>>   1 file changed, 15 insertions(+), 16 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> >>> b/drivers/gpu/drm/i915/intel_hdmi.c
> >>> index 5ccb305a6e1c..4760462f84ca 100644
> >>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >>> @@ -963,7 +963,9 @@ static void 
> >>> intel_hdmi_compute_gcp_infoframe(struct intel_encoder *encoder,
> >>> intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GENERAL_CONTROL);
> >>>         /* Indicate color depth whenever the sink supports deep 
> >>> color */
> >>> -    if (hdmi_sink_is_deep_color(conn_state))
> >>> +
> >>> +    if (hdmi_sink_is_deep_color(conn_state) &&
> >>> +        (crtc_state->pipe_bpp > 24))
> >>>           crtc_state->infoframes.gcp |= GCP_COLOR_INDICATION;
> >>>         /* Enable default_phase whenever the display mode is 
> >>> suitably aligned */

So I think this hunk on its own should be sufficient. I'd like to see
the pointless parens eliminated, and a comment added that explains
that 8bpc + color depth indication is no longer supported on HSW+.

> >>> @@ -2260,6 +2262,7 @@ int intel_hdmi_compute_config(struct 
> >>> intel_encoder *encoder,
> >>>       int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
> >>>       int clock_10bpc = clock_8bpc * 5 / 4;
> >>>       int clock_12bpc = clock_8bpc * 3 / 2;
> >>> +    int dc_clock = clock_12bpc;
> >>>       int desired_bpp;
> >>>       bool force_dvi = intel_conn_state->force_audio == 
> >>> HDMI_AUDIO_OFF_DVI;
> >>>   @@ -2314,22 +2317,18 @@ int intel_hdmi_compute_config(struct 
> >>> intel_encoder *encoder,
> >>>        * Note that g4x/vlv don't support 12bpc hdmi outputs. We also 
> >>> need
> >>>        * to check that the higher clock still fits within limits.
> >>>        */
> >>> -    if (hdmi_deep_color_possible(pipe_config, 12) &&
> >>> -        hdmi_port_clock_valid(intel_hdmi, clock_12bpc,
> >>> +    if (pipe_config->pipe_bpp == 30)
> >>> +        dc_clock = clock_10bpc;
> >>> +
> >>> +    if (hdmi_deep_color_possible(pipe_config, pipe_config->pipe_bpp 
> >>> / 3) &&
> >>> +        hdmi_port_clock_valid(intel_hdmi, dc_clock,
> >>>                     true, force_dvi) == MODE_OK) {
> >>> -        DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
> >>> -        desired_bpp = 12*3;
> >>> -
> >>> -        /* Need to adjust the port link by 1.5x for 12bpc. */
> >>> -        pipe_config->port_clock = clock_12bpc;
> >>> -    } else if (hdmi_deep_color_possible(pipe_config, 10) &&
> >>> -           hdmi_port_clock_valid(intel_hdmi, clock_10bpc,
> >>> -                     true, force_dvi) == MODE_OK) {
> >>> -        DRM_DEBUG_KMS("picking bpc to 10 for HDMI output\n");
> >>> -        desired_bpp = 10 * 3;
> >>> -
> >>> -        /* Need to adjust the port link by 1.25x for 10bpc. */
> >>> -        pipe_config->port_clock = clock_10bpc;
> >>> +        DRM_DEBUG_KMS("picking bpc to %d for HDMI output\n",
> >>> +                 pipe_config->pipe_bpp / 3);
> >>> +        desired_bpp = pipe_config->pipe_bpp;
> >>> +
> >>> +        /* Need to adjust the port link dc modes. */
> >>> +        pipe_config->port_clock = dc_clock;
> >> This part has nothing to do with the commit message.
> >>
> >>
> >>>       } else {
> >>>           DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
> >>>           desired_bpp = 8*3;
> >>> -- 
> >>> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color
URL   : https://patchwork.freedesktop.org/series/58870/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
925957d2a043 drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color
-:36: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'crtc_state->pipe_bpp > 24'
#36: FILE: drivers/gpu/drm/i915/intel_hdmi.c:967:
+	if (hdmi_sink_is_deep_color(conn_state) &&
+	    (crtc_state->pipe_bpp > 24))

-:75: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#75: FILE: drivers/gpu/drm/i915/intel_hdmi.c:2327:
+		DRM_DEBUG_KMS("picking bpc to %d for HDMI output\n",
+				 pipe_config->pipe_bpp / 3);

total: 0 errors, 0 warnings, 2 checks, 50 lines checked
3e78da0fe30e drm/i915: Add N & CTS values for 10/12 bit deep color

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color
URL   : https://patchwork.freedesktop.org/series/58870/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5856 -> Patchwork_12657
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58870/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12657 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@fork-gfx0:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@basic-bsd2:
    - fi-kbl-7500u:       NOTRUN -> SKIP [fdo#109271] +9
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_basic@readonly-bsd1:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-allowed:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109289] +1

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109289] +1

  * igt@gem_exec_suspend@basic-s3:
    - fi-skl-6700k2:      PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_pm_rpm@module-reload:
    - fi-icl-y:           NOTRUN -> INCOMPLETE [fdo#108840]

  * igt@i915_selftest@live_contexts:
    - fi-icl-u2:          NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         NOTRUN -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       NOTRUN -> DMESG-WARN [fdo#103841]
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109284] +8

  * igt@kms_chamelium@dp-edid-read:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109316] +2

  * igt@kms_chamelium@vga-hpd-fast:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109309] +1

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_psr@primary_mmap_gtt:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#110189] +3
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +23

  * igt@kms_psr@primary_page_flip:
    - fi-apl-guc:         NOTRUN -> SKIP [fdo#109271] +50

  * igt@prime_vgem@basic-fence-flip:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109294]

  * igt@runner@aborted:
    - fi-kbl-7500u:       NOTRUN -> FAIL [fdo#103841]
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  * igt@i915_selftest@live_uncore:
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110210] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
    - fi-byt-clapper:     FAIL [fdo#107362] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     INCOMPLETE [fdo#102657] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-u3:          INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569]

  
  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109316]: https://bugs.freedesktop.org/show_bug.cgi?id=109316
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (43 -> 42)
------------------------------

  Additional (4): fi-icl-y fi-icl-u2 fi-apl-guc fi-kbl-7500u 
  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-skl-6260u fi-icl-dsi fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5856 -> Patchwork_12657

  CI_DRM_5856: 55074bd825098a71779cf65a69786547f0eccbe9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4922: e941e4a29438c7130554492e4daf52afbc99ffdf @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12657: 3e78da0fe30e0274447b515730da2b3e7f4f8f28 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3e78da0fe30e drm/i915: Add N & CTS values for 10/12 bit deep color
925957d2a043 drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12657/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [v2,1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color
URL   : https://patchwork.freedesktop.org/series/58870/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5856_full -> Patchwork_12657_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12657_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@preempt-self-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276]

  * igt@gem_exec_store@cachelines-bsd2:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +20

  * igt@gem_pwrite@stolen-normal:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +140

  * igt@gem_render_copy_redux@flink-interruptible:
    - shard-snb:          NOTRUN -> INCOMPLETE [fdo#105411]

  * igt@gem_stolen@stolen-no-mmap:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277]

  * igt@i915_pm_sseu@full-enable:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109288]

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@kms_atomic_transition@3x-modeset-transitions:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +13

  * igt@kms_busy@basic-modeset-e:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-modeset-hang-newfb-render-e:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#110222] +1

  * igt@kms_concurrent@pipe-f:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +1

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          NOTRUN -> FAIL [fdo#103833]

  * igt@kms_flip@2x-wf_vblank-ts-check:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +39

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          PASS -> FAIL [fdo#105363]

  * igt@kms_force_connector_basic@force-connector-state:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109285]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-gtt:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +7

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +21

  * igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
    - shard-skl:          NOTRUN -> FAIL [fdo#105683]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-render:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_lease@atomic_implicit_crtc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#110279]
    - shard-skl:          NOTRUN -> FAIL [fdo#110279]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145] +2
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         PASS -> SKIP [fdo#109642]

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]
    - shard-iclb:         NOTRUN -> FAIL [fdo#99912]
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-iclb:         PASS -> FAIL [fdo#104894]

  
#### Possible fixes ####

  * igt@gem_create@create-clear:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-iclb:         INCOMPLETE [fdo#109801] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-apl:          DMESG-WARN [fdo#110222] -> PASS

  * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          FAIL [fdo#106509] / [fdo#107409] -> PASS

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
    - shard-iclb:         FAIL [fdo#103355] -> PASS

  * igt@kms_flip_tiling@flip-to-x-tiled:
    - shard-iclb:         FAIL [fdo#108134] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +18

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-wc:
    - shard-iclb:         FAIL [fdo#105682] / [fdo#109247] -> PASS

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +3

  * igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr@no_drrs:
    - shard-iclb:         FAIL [fdo#108341] -> PASS

  * igt@kms_psr@psr2_primary_mmap_gtt:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +2

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-apl:          FAIL [fdo#104894] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-iclb:         FAIL [fdo#104894] -> PASS

  
#### Warnings ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         DMESG-WARN [fdo#108686] -> FAIL [fdo#108686]

  * igt@i915_pm_rpm@modeset-non-lpsp:
    - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP [fdo#109271]

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103833]: https://bugs.freedesktop.org/show_bug.cgi?id=103833
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#108134]: https://bugs.freedesktop.org/show_bug.cgi?id=108134
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109288]: https://bugs.freedesktop.org/show_bug.cgi?id=109288
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5856 -> Patchwork_12657

  CI_DRM_5856: 55074bd825098a71779cf65a69786547f0eccbe9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4922: e941e4a29438c7130554492e4daf52afbc99ffdf @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12657: 3e78da0fe30e0274447b515730da2b3e7f4f8f28 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12657/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx