diff mbox series

drm/i915: Fix setting 10 bit color mode

Message ID 20190409195858.26566-1-aditya.swarup@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Fix setting 10 bit color mode | expand

Commit Message

Aditya Swarup April 9, 2019, 7:58 p.m. UTC
Currently, we cannot set 10 bit color mode in
intel_hdmi_compute_config() because desired_bpp is always set to 12
which makes pipe_bpp = 36.(As most platforms support 12 bit color which
always returns true for hdmi_deep_color_possible() in the if block for
12 bit color)

pipe_bpp value is always current and we don't need to determine the
correct settings again using desired_bpp in intel_hdmi_compute_config().
So, use the value of pipe_bpp to determine whether the current settings
for color are applicable for the platform and change the port clock
accordingly. This makes the code generic and handles each case with a
single call to hdmi_deep_color_possible().

Co-authored-by: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 45 +++++++++++++++----------------
 1 file changed, 21 insertions(+), 24 deletions(-)

Comments

Ville Syrjälä April 9, 2019, 8:15 p.m. UTC | #1
On Tue, Apr 09, 2019 at 12:58:58PM -0700, Aditya Swarup wrote:
> Currently, we cannot set 10 bit color mode in
> intel_hdmi_compute_config() because desired_bpp is always set to 12
> which makes pipe_bpp = 36.(As most platforms support 12 bit color which
> always returns true for hdmi_deep_color_possible() in the if block for
> 12 bit color)
> 
> pipe_bpp value is always current and we don't need to determine the
> correct settings again using desired_bpp in intel_hdmi_compute_config().
> So, use the value of pipe_bpp to determine whether the current settings
> for color are applicable for the platform and change the port clock
> accordingly. This makes the code generic and handles each case with a
> single call to hdmi_deep_color_possible().
> 
> Co-authored-by: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 45 +++++++++++++++----------------
>  1 file changed, 21 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index f2c0aba4371b..dba0ec079ba2 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2263,7 +2263,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 desired_bpp;
> +	int clock_bpc;
>  	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
>  
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> @@ -2317,32 +2317,29 @@ 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 == 36) {
> +		/* Need to adjust clock by 1.5x for 12bpc. */
> +		clock_bpc = clock_12bpc;
> +	} else if (pipe_config->pipe_bpp == 30) {
> +		/* Need to adjust clock by 1.25x for 10bpc. */
> +		clock_bpc = clock_10bpc;
> +	} else
> +		clock_bpc = clock_8bpc;

This doesn't seem to allow graceful fallback from 12 to 10 to 8.

Did you try the oneliner I suggested?

> +
> +	if (!pipe_config->bw_constrained) {
> +		if (pipe_config->pipe_bpp > 24 &&
> +		    hdmi_deep_color_possible(pipe_config, pipe_config->pipe_bpp) &&
> +		    hdmi_port_clock_valid(intel_hdmi, clock_bpc,
>  				  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;
> -	} else {
> -		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
> -		desired_bpp = 8*3;
> +			DRM_DEBUG_KMS("picking bpc to %d for HDMI output\n",pipe_config->pipe_bpp/3);
>  
> -		pipe_config->port_clock = clock_8bpc;
> -	}
> +			pipe_config->port_clock = clock_bpc;
> +		} else {
> +			DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
> +			pipe_config->pipe_bpp = 24;
>  
> -	if (!pipe_config->bw_constrained) {
> -		DRM_DEBUG_KMS("forcing pipe bpp to %i for HDMI\n", desired_bpp);
> -		pipe_config->pipe_bpp = desired_bpp;
> +			pipe_config->port_clock = clock_8bpc;
> +		}
>  	}
>  
>  	if (hdmi_port_clock_valid(intel_hdmi, pipe_config->port_clock,
> -- 
> 2.17.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index f2c0aba4371b..dba0ec079ba2 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2263,7 +2263,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 desired_bpp;
+	int clock_bpc;
 	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
@@ -2317,32 +2317,29 @@  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 == 36) {
+		/* Need to adjust clock by 1.5x for 12bpc. */
+		clock_bpc = clock_12bpc;
+	} else if (pipe_config->pipe_bpp == 30) {
+		/* Need to adjust clock by 1.25x for 10bpc. */
+		clock_bpc = clock_10bpc;
+	} else
+		clock_bpc = clock_8bpc;
+
+	if (!pipe_config->bw_constrained) {
+		if (pipe_config->pipe_bpp > 24 &&
+		    hdmi_deep_color_possible(pipe_config, pipe_config->pipe_bpp) &&
+		    hdmi_port_clock_valid(intel_hdmi, clock_bpc,
 				  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;
-	} else {
-		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
-		desired_bpp = 8*3;
+			DRM_DEBUG_KMS("picking bpc to %d for HDMI output\n",pipe_config->pipe_bpp/3);
 
-		pipe_config->port_clock = clock_8bpc;
-	}
+			pipe_config->port_clock = clock_bpc;
+		} else {
+			DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
+			pipe_config->pipe_bpp = 24;
 
-	if (!pipe_config->bw_constrained) {
-		DRM_DEBUG_KMS("forcing pipe bpp to %i for HDMI\n", desired_bpp);
-		pipe_config->pipe_bpp = desired_bpp;
+			pipe_config->port_clock = clock_8bpc;
+		}
 	}
 
 	if (hdmi_port_clock_valid(intel_hdmi, pipe_config->port_clock,