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

From: Jason Wessel <jason.wessel@windriver.com>
To: Jesse Barnes <jbarnes@virtuousgeek.org>,
	Paulo Zanoni <paulo.r.zanoni@intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: Re: [PATCH 12/13] drm/i915: remove in_dbg_master check from intel_fbc.c
Date: Wed, 4 Nov 2015 14:19:36 -0600	[thread overview]
Message-ID: <563A6858.4080007@windriver.com> (raw)
In-Reply-To: <563A66FF.6090101@virtuousgeek.org>

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.

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