Re: [PATCH 2/2] drm/i915: improve assert_panel_unlocked

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Aug 22, 2014 at 11:00:15AM +0300, Ville Syrjälä wrote:
> On Thu, Aug 21, 2014 at 12:01:07PM -0300, Paulo Zanoni wrote:
> > 2014-08-21 11:56 GMT-03:00 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> > > On Thu, Aug 21, 2014 at 03:06:26PM +0300, Jani Nikula wrote:
> > >> Fix assert_panel_unlocked for vlv/chv, and improve it a bit for
> > >> non-LVDS. Also don't pretend it works for DDI. There's still work to do
> > >> to get this right for eDP on PCH platforms, but this is a start.
> > >>
> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > >>
> > >> ---
> > >>
> > >> So I wanted to quickly fix assert_panel_unlocked, but for such a short
> > >> piece of code it's too involved to _quickly_ get right across all
> > >> platforms. I think this is a worthwhile improvement though.
> > >> ---
> > >>  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++-------
> > >>  1 file changed, 20 insertions(+), 7 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > >> index fe1d00dc9ef5..d6b48496d7f4 100644
> > >> --- a/drivers/gpu/drm/i915/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/intel_display.c
> > >> @@ -1193,17 +1193,33 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
> > >>  static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
> > >>                                 enum pipe pipe)
> > >>  {
> > >> -     int pp_reg, lvds_reg;
> > >> +     struct drm_device *dev = dev_priv->dev;
> > >> +     int pp_reg;
> > >>       u32 val;
> > >>       enum pipe panel_pipe = PIPE_A;
> > >>       bool locked = true;
> > >>
> > >> -     if (HAS_PCH_SPLIT(dev_priv->dev)) {
> > >> +     if (HAS_DDI(dev)) {
> > >> +             /* XXX: this neither works nor gets called for DDI */
> > >
> > > Not sure why the XXX here. Seems to me there's nothing to fix here for
> > > DDI. Maybe just make that a WARN_ON(HAS_DDI()) or just remove the check
> > > entirely.
> > 
> > As far as I remember, the "abcd" stuff is not even used/needed on DDI.
> > But this is just what my memory tells me, it may be wrong. Someone
> > needs to double-check.
> 
> Bspec just says "spare" for those bits.

Iirc it's also not used for edp on older platforms, but only for lvds.
There's definitely a lot of fun in this area ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch