[Intel-gfx] [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 at 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 at intel.com>
>> Signed-off-by: Aditya Swarup <aditya.swarup at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Cc: Manasi Navare <manasi.d.navare at 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