diff mbox series

[07/12] drm/i915: Don't look at unrelated PIPECONF bits for interlaced readout

Message ID 20190718145053.25808-8-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: YCbCr output fixes and prep work for YCbCr 4:4:4 output | expand

Commit Message

Ville Syrjälä July 18, 2019, 2:50 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Since HSW the PIPECONF progressive vs. interlaced selection is done
with just two bits instead of the earlier three. Let's not look at the
extra bit on HSW+. Also gen2 doesn't support interlaced displays at all.

This is actually fine as is currently because the extra bit is mbz (as
are all three bits on gen2). But just to avoid mishaps in the future
if the bits get reused let's only look at what's properly defined.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Gwan-gyeong Mun Sept. 18, 2019, 7:02 p.m. UTC | #1 
On Thu, 2019-07-18 at 17:50 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since HSW the PIPECONF progressive vs. interlaced selection is done
> with just two bits instead of the earlier three. Let's not look at
> the
> extra bit on HSW+. Also gen2 doesn't support interlaced displays at
> all.
> 
> This is actually fine as is currently because the extra bit is mbz
> (as
> are all three bits on gen2). But just to avoid mishaps in the future
> if the bits get reused let's only look at what's properly defined.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index e25b82d07d4f..ffdc350dc24a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8189,6 +8189,21 @@ static void intel_set_pipe_src_size(const
> struct intel_crtc_state *crtc_state)
>  		   (crtc_state->pipe_src_h - 1));
>  }
>  
> +static bool intel_pipe_is_interlaced(struct intel_crtc_state
> *crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc_state-
> >base.crtc->dev);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +
> +	if (IS_GEN(dev_priv, 2))
> +		return false;
> +
> +	if (INTEL_GEN(dev_priv) >= 9 ||
> +	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> +		return I915_READ(PIPECONF(cpu_transcoder)) &
> PIPECONF_INTERLACE_MASK_HSW;
> +	else
> +		return I915_READ(PIPECONF(cpu_transcoder)) &
> PIPECONF_INTERLACE_MASK;
> +}
> +
>  static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  				   struct intel_crtc_state
> *pipe_config)
>  {
> @@ -8227,7 +8242,7 @@ static void intel_get_pipe_timings(struct
> intel_crtc *crtc,
>  	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp &
> 0xffff) + 1;
>  	pipe_config->base.adjusted_mode.crtc_vsync_end = ((tmp >> 16) &
> 0xffff) + 1;
>  
> -	if (I915_READ(PIPECONF(cpu_transcoder)) &
> PIPECONF_INTERLACE_MASK) {
> +	if (intel_pipe_is_interlaced(pipe_config)) {
>  		pipe_config->base.adjusted_mode.flags |=
> DRM_MODE_FLAG_INTERLACE;
>  		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
>  		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
The changes look good to me.
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e25b82d07d4f..ffdc350dc24a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8189,6 +8189,21 @@  static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state)
 		   (crtc_state->pipe_src_h - 1));
 }
 
+static bool intel_pipe_is_interlaced(struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+
+	if (IS_GEN(dev_priv, 2))
+		return false;
+
+	if (INTEL_GEN(dev_priv) >= 9 ||
+	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+		return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK_HSW;
+	else
+		return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK;
+}
+
 static void intel_get_pipe_timings(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config)
 {
@@ -8227,7 +8242,7 @@  static void intel_get_pipe_timings(struct intel_crtc *crtc,
 	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
 	pipe_config->base.adjusted_mode.crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
 
-	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
+	if (intel_pipe_is_interlaced(pipe_config)) {
 		pipe_config->base.adjusted_mode.flags |= DRM_MODE_FLAG_INTERLACE;
 		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
 		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;