Re: [PATCH 12/13] drm/i915: remove in_dbg_master check from intel_fbc.c

["Zanoni, Paulo R" <paulo.r.zanoni@intel.com>]

Em Qua, 2015-11-04 às 14:19 -0600, Jason Wessel escreveu:
> On 11/04/2015 02:13 PM, Jesse Barnes wrote:
> > On 11/04/2015 11:10 AM, Paulo Zanoni wrote:
> > >  From our maintainer Daniel Vetter a few days ago:
> > >    "Oh dear this is dead code. kdbg uses the fbcon, which always
> > > uses
> > >    untiled, which means fbc will never be enabled. Also we have 0
> > > users
> > >    and 0 test coverage for kdbg on top of i915 (Jesse implemented
> > > it
> > >    for fun years back). Imo just remove all this code."
> > > 
> > > Adding to what Daniel said: for kgdboc's KMS support,
> > > intel_pipe_set_base_atomic() already manually disables FBC, so we
> > > won't do the in_dbg_master() check there. This is essentially a
> > > revert
> > > of:
> > > 
> > > commit c924b934d0cd14a4559611da91f28f59acebe32a
> > > Author: Jason Wessel <jason.wessel@windriver.com>
> > > Date:   Thu Aug 5 09:22:32 2010 -0500
> > >      i915: when kgdb is active display compression should be off
> > > 
> > > Besides, it is not clear what is the exact problem caused by FBC,
> > > and
> > > why other features such as PSR, DRRS, IPS and RPM are not also
> > > checking for in_dbg_master(). IMHO we should either remove the
> > > code as
> > > suggested by Daniel or we add some nice comments explaining why
> > > is FBC
> > > so special.
> > > 
> > > v2: Rebase due to new patch order.
> > > 
> > > Cc: Jason Wessel <jason.wessel@windriver.com>
> > > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_fbc.c | 6 ------
> > >   1 file changed, 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > > b/drivers/gpu/drm/i915/intel_fbc.c
> > > index 8e806be..e496cb0 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > @@ -890,12 +890,6 @@ static void __intel_fbc_update(struct
> > > drm_i915_private *dev_priv)
> > >   		goto out_disable;
> > >   	}
> > >   
> > > -	/* If the kernel debugger is active, always disable
> > > compression */
> > > -	if (in_dbg_master()) {
> > > -		set_no_fbc_reason(dev_priv, "Kernel debugger is
> > > active");
> > > -		goto out_disable;
> > > -	}
> > > -
> > >   	/* WaFbcExceedCdClockThreshold:hsw,bdw */
> > >   	if ((IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) &&
> > >   	    ilk_pipe_pixel_rate(crtc->config) >=
> > > 
> > Yeah looks fine.  I haven't had any bug reports from the kdboc
> > work, so
> > I guess that means no one is using it. :)
> > 
> > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> 
> It was previously the case that the code here only got hit when you
> had a oops or a panic while running with the graphics console up. We
> would end up with fuzzy lines instead of a readable text console when
> we activated the atomic mode set.

But on this case we'll call pipe_set_base_atomic(), which will disable
FBC before doing the modeset. Maybe this was not the case in the
past..?

> 
> The kgdboc probably doesn't get used a whole lot but I have debugged
> a few strange WiFi and PM problems with it on a laptop and it worked
> well.  :-)


> Cheers,
> Jason.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx