Re: [PATCH 0/4] Enable FBC on SKL, v3

["Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com>]

On 4/6/2016 7:24 PM, Zanoni, Paulo R wrote:
> Em Qua, 2016-04-06 às 10:36 +0530, Thulasimani, Sivakumar escreveu:
>> dont want to hijack thread but wanted to point out a possible
>> regression
>> in the
>> previous patches of this series.
>>
>> intel_fbc_can_choose: returns true for gen 4/5/6/7. (possible bug)
> How? It will check for i915.enable_fbc, which will have been sanitized
> to zero on these platforms. Aren't you explicitly enabling FBC on these
> platforms by using i915.enable_fbc=1?
nope, found as part of code walkthrough hence mentioned above that it is
"possible regression" :) .

Two points
 > found that this is not a regression per say as it was there before 
your change too
 > Agree that FBC code wont reach intel_fbc_can_choose due to platform 
checks
   but without proper comments wont someone assume that the rest of code 
should
work properly especially when we have checks for non haswell/bdw 
platforms in
the code flow.

Also found another possible bug in multiple_pipes_ok.
i assume it is supposed to return true when we can enable FBC
after considering multiple pipes are enabled or not.
but it returns false in case of single display in gen3 thus ending up
doing opposite of multiple pipe requirement.

i understand all these are on very old platforms which dont have
FBC enabled at all so fixing them should be lower priority.
but sharing them here so that at-least it is documented somewhere.

regards,
Sivakumar
>> so intel_crtc_state->enable_fbc = true; will be executed for first
>> crtc
>> everytime intel_fbc_choose_crtc is called. Although there is check to
>> handle fbc already enabled, it may fail when we fbc is disabled and
>> we are working on non supported panel.
>>
>> regards,
>> Sivakumar
>>
>> On 4/5/2016 2:47 AM, Paulo Zanoni wrote:
>>> Now with the suggestion from Chris instead of the old workaround.
>>> We don't need
>>> new DDX patches anymore, but now we need new IGT patches.
>>>
>>> Chris Wilson (1):
>>>     drm/i915: use ORIGIN_CPU for frontbuffer invalidation on WC
>>> mmaps
>>>
>>> Paulo Zanoni (3):
>>>     drm/i915/fbc: update busy_bits even for GTT and flip flushes
>>>     drm/i915/fbc: sanitize i915.enable_fbc during FBC init
>>>     drm/i915/fbc: enable FBC on gen 9+ too
>>>
>>>    drivers/gpu/drm/i915/i915_drv.h  |  1 +
>>>    drivers/gpu/drm/i915/i915_gem.c  | 14 +++++++++++---
>>>    drivers/gpu/drm/i915/intel_fbc.c | 27 ++++++++++++++++-----------
>>>    3 files changed, 28 insertions(+), 14 deletions(-)
>>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx