Re: [PATCH] drm/i915: Use the correct PCH transcoder for LPT/WPT in intel_sanitize_frame_start_delay()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Nov 29, 2019 at 11:56:42AM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Shankar, Uma
> >Sent: Friday, November 29, 2019 5:18 PM
> >To: Ville Syrjala <ville.syrjala@linux.intel.com>; intel-gfx@lists.freedesktop.org
> >Subject: RE: [PATCH] drm/i915: Use the correct PCH transcoder for LPT/WPT in
> >intel_sanitize_frame_start_delay()
> >
> >
> >
> >>-----Original Message-----
> >>From: Ville Syrjala <ville.syrjala@linux.intel.com>
> >>Sent: Thursday, November 28, 2019 11:54 PM
> >>To: intel-gfx@lists.freedesktop.org
> >>Cc: Shankar, Uma <uma.shankar@intel.com>
> >>Subject: [PATCH] drm/i915: Use the correct PCH transcoder for LPT/WPT
> >>in
> >>intel_sanitize_frame_start_delay()
> >>
> >>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >>LPT/WPT only have PCH transcoder A. Make sure we poke at its chicken
> >>register instead of some non-existent register when FDI is being driven by pipe B or
> >C.
> >
> >Change looks good to me.
> >Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> >
> >>Cc: Uma Shankar <uma.shankar@intel.com>
> >>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>---
> >> drivers/gpu/drm/i915/display/intel_display.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> >>b/drivers/gpu/drm/i915/display/intel_display.c
> >>index 53dc310a5f6d..f99dbc3d9696 100644
> >>--- a/drivers/gpu/drm/i915/display/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/display/intel_display.c
> >>@@ -17272,7 +17272,8 @@ static void
> >>intel_sanitize_frame_start_delay(const struct intel_crtc_state *crtc
> >> 		val |= TRANS_FRAME_START_DELAY(0);
> >> 		I915_WRITE(reg, val);
> >> 	} else {
> >>-		i915_reg_t reg = TRANS_CHICKEN2(crtc->pipe);
> >>+		enum pipe pch_transcoder = intel_crtc_pch_transcoder(crtc);
> 
> Just an afterthought, not sure if this hold generically for all pipes or is it programmable
> only for PIPE_A. Making it enabled on PIPE_A when actually pipe B or C is used, is it the right
> thing. Should we discard programming this for PIPE B and C altogether.

This is about PCH transcoder A instead of pipe A despite us using
enum pipe here. We just happen to use enum pipe for PCH transcoders
since it was easier than adding a new enum for it.

So it's perfectly legal to have any of these configurations on HSW/BDW:
pipe A -> CPU transcoder A -> DDI E -> FDI -> PCH transcoder A -> CRT
pipe B -> CPU transcoder B -> DDI E -> FDI -> PCH transcoder A -> CRT
pipe C -> CPU transcoder C -> DDI E -> FDI -> PCH transcoder A -> CRT

So we want to do this for any pipe when has_pch_encoder indicates
that the data is going over FDI to the PCH.

> 
> >>+		i915_reg_t reg = TRANS_CHICKEN2(pch_transcoder);
> >> 		u32 val;
> >>
> >> 		val = I915_READ(reg);
> >>--
> >>2.23.0
> 

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