On 11/04/2015 12:26 PM, Zanoni, Paulo R wrote:
> 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@xxxxxxxxxxxxx>
>>>> 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@xxxxxxxxxxxxx>
>>>> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
>>>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
>>>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>>>> ---
>>>> 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@xxxxxxxxxxxxxxxx>
>>
>>
>> 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..?
Or some subsequent call re-enabled it somehow. You could replace it
with a WARN and keep my r-b if you want, or just push as-is.
But my guess is that kgdboc hasn't survived all the other modeset work
we've done the past couple of years... I guess we need some igt tests
and a bunch of fixups to keep it going. We could address this problem
if/when that happens.
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
