Re: [PATCH 0/7] drm/i915: Move DP link parameters out from intel_dp

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

On Tue, Aug 25, 2015 at 02:57:45PM +0200, Daniel Vetter wrote:
> On Mon, Aug 17, 2015 at 03:03:10PM +0300, Ville Syrjälä wrote:
> > On Mon, Aug 17, 2015 at 10:42:23AM +0200, Maarten Lankhorst wrote:
> > > Hey,
> > > 
> > > Op 06-07-15 om 14:09 schreef ville.syrjala@linux.intel.com:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >
> > > > While working on CHV DPIO powergating I relized DP .compute_config() was
> > > > clobbering lane_count etc. stored in intel_dp. This could cause problems
> > > > if we do the .compute_config() but later fail the modeset for some reason.
> > > > Any subsequent link re-training might then fail if intel_dp->lane_count
> > > > etc. got changed.
> > > >
> > > > The reason I ran into this during the DPIO powergating work was that I may
> > > > need to know which lanes he active when shutting down the link. However
> > > > .compute_config() already clobbered that information by the time I need it.
> > > > By moving it to the pipe config we avoid that problem as well.
> > > >
> > > > I also cleaned up the limited color range handling a bit while I was
> > > > in the neighborhood.
> > > >
> > > > Ville Syrjälä (7):
> > > >   drm/i915: Clean up DP/HDMI limited color range handling
> > > >   drm/i915: Don't use link_bw for PLL setup
> > > ^
> > > >   drm/i915: Don't pass clock to DDI PLL select functions
> > > >   drm/i915: Avoid confusion between DP and TRANS_DP_CTL in DP
> > > >     .get_config()
> > > >   drm/i915: Move intel_dp->lane_count into pipe_config
> > > ^Your patch series breaks MST entirely.
> > > 
> > > drm_encoder->crtc will be NULL and dereferencing crtc->config will fail on the primary encoder:
> > > 
> > > intel_mst_pre_enable_dp(mst_enc) -> intel_ddi_init_dp_buf_reg(mst_enc->primary). -> primary->crtc = NULL, OOPS.
> > > 
> > > Voltage training was failing too and I was getting a MCE until I reverted the crtc->config changes from this series..
> > 
> > Hmm. Our modeset code vs. mst seems to a huge mess. To fix it properly
> > we should split the link compute/setup from the pipe config entirely.
> > As a short term solution I suppose I could pass the pipe config into
> > various dp link setup functions rather than assume that we can dig it
> > out from the encoder->crtc.
> 
> Luckily seris is in 4.4 only. Should I just drop it, or is it easily
> salvageable?

Fix is on the list with Maarten's t-b.

> -Daniel
> 
> > 
> > > 
> > > 
> > > >   drm/i915: Don't use link_bw to select between TP1 and TP3
> > > >   drm/i915: Kill intel_dp->{link_bw,rate_select}
> > > >
> > > >  drivers/gpu/drm/i915/i915_reg.h      |   3 +
> > > >  drivers/gpu/drm/i915/intel_ddi.c     |  41 ++++----
> > > >  drivers/gpu/drm/i915/intel_display.c |   7 +-
> > > >  drivers/gpu/drm/i915/intel_dp.c      | 177 ++++++++++++++++++++---------------
> > > >  drivers/gpu/drm/i915/intel_dp_mst.c  |  19 ++--
> > > >  drivers/gpu/drm/i915/intel_drv.h     |   9 +-
> > > >  drivers/gpu/drm/i915/intel_hdmi.c    |  26 +++--
> > > >  7 files changed, 155 insertions(+), 127 deletions(-)
> > > >
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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