>-----Original Message----- >From: Ville Syrjälä <ville.syrj...@linux.intel.com> >Sent: Friday, November 29, 2019 7:21 PM >To: Shankar, Uma <uma.shan...@intel.com> >Cc: 'intel-gfx@lists.freedesktop.org' <intel-gfx@lists.freedesktop.org> >Subject: Re: [PATCH] drm/i915: Use the correct PCH transcoder for LPT/WPT in >intel_sanitize_frame_start_delay() > >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.syrj...@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.syrj...@linux.intel.com> >> >>Sent: Thursday, November 28, 2019 11:54 PM >> >>To: intel-gfx@lists.freedesktop.org >> >>Cc: Shankar, Uma <uma.shan...@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.syrj...@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.shan...@intel.com> >> > >> >>Cc: Uma Shankar <uma.shan...@intel.com> >> >>Signed-off-by: Ville Syrjälä <ville.syrj...@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.
Yeah I was thinking on same line that PIPE_A is just a means to get PCH TRANSCODER rather than having any significance with pipe here. Thanks for clarying this Ville. This patch is good from my perspective. Regards, Uma Shankar >> >> >>+ 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