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@xxxxxxxxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >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@xxxxxxxxxxxxxxx> > >>Sent: Thursday, November 28, 2019 11:54 PM > >>To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >>Cc: Shankar, Uma <uma.shankar@xxxxxxxxx> > >>Subject: [PATCH] drm/i915: Use the correct PCH transcoder for LPT/WPT > >>in > >>intel_sanitize_frame_start_delay() > >> > >>From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >> > >>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@xxxxxxxxx> > > > >>Cc: Uma Shankar <uma.shankar@xxxxxxxxx> > >>Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >>--- > >> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx