[Intel-gfx] [PATCH] drm/i915: Avoid reading fbc registers in vain when fbc was never enabled.

[Paulo Zanoni]

2014-09-19 15:24 GMT-03:00 Rodrigo Vivi <rodrigo.vivi at gmail.com>:
>
>
> On Fri, Sep 19, 2014 at 8:40 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
>>
>> On Thu, Sep 18, 2014 at 11:42:44AM -0700, Rodrigo Vivi wrote:
>> > On Thu, Sep 18, 2014 at 4:39 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
>> >
>> > > On Thu, Sep 18, 2014 at 07:28:48AM +0100, Chris Wilson wrote:
>> > > > On Wed, Sep 17, 2014 at 04:59:20PM -0400, Rodrigo Vivi wrote:
>> > > > > If it wasn't never enabled by kernel parameter or platform default
>> > > > > we can avoid reading registers so many times in vain
>> > > >
>> > > > Nak.
>> > >
>> > > Well I've merged this for now to reduce fbc impact.
>> > >
>> >
>> > Uhm, unfortunatelly I'm afraid Chris was right.
>> > Paulo also nacked it. Because it just helps when it was explicitly
>> > disabled
>> > by setting i915.enable_fbc=0 while the default is -1.
>> >
>> > I though about returning on <= 0, but Paulo is afraid that when enabling
>> > back for some platform people would forget to fix this part here and I
>> > agree.
>>
>> Well I guess I should have read mails before pushing out a new -next ;-)
>> So this is now baked in. Should I revert or can we just fix up on top?
>
>
> No problem, I can fix this on top.
>
> But what do you prefer:
>
> 1. <=0 return and changing parameter permission from 600 to 400

If i915.enable_fbc is -1, you don't know whether it's enabled or not.

> 2. dev_priv->fbc_enabled

I was just quickly tried to write this patch. It fixes the runtime PM
regression I was seeing...

> 3. or check if fbc_no_reason was ever set? Although I don't like the
> fbc_no_reason and would like to clean it up in the future. But anyway, any
> option here is temporary until a proper rework for cleanup and real fix.

I also tried the fbc_no_reason path but gave up. When no_fbc_reason is
FBC_OK, there's no guarantee that FBC is actually enabled: it just
mean that we may be enabling/disabling it as part of the normal FBC
operation. So to do what we want, we'd have to twist the meaning of
no_fbc_reason and add values like FBC_OK_AND_ENABLED and
FBC_OK_AND_DISABLED, and I don't think that's a cool solution.

I'll send the patch for item 2 in a few minutes.

>
>>
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>



-- 
Paulo Zanoni