[Intel-gfx] [PATCH 02/12] drm/i915: set dev_priv->fbc.crtc before scheduling the enable work

[Daniel Stone]

Hi,

On 13 November 2015 at 21:13, Paulo Zanoni <przanoni at gmail.com> wrote:
> 2015-11-13 18:56 GMT-02:00 Chris Wilson <chris at chris-wilson.co.uk>:
>> This is confusing me. I think of FBC in terms of the CRTC/pipe, and the
>> fb just describes the plane currently bound to the primary. You've
>> pushed everywhere else to also work on the CRTC, I think it would be
>> consistent here to pass crtc and then use fbc.fb_id =
>> crtc->primary->fb->id.
>>
>> Have I missed something in the later patches that explains the choice
>> here?
>
> You are right that this is confusing. This is basically an artifact
> from the previous FBC design that I overlooked. I guess during the
> conversion, my thinking was "well, the arguments are the stuff we
> store in the work struct, and since we don't store the CRTC anymore,
> let's remove it and keep passing the FB since it's still part of the
> work struct". Now that you bring this under a different perspective, I
> see the problem. We don't even need to pass these things as arguments
> since we do check that work->fb is equal to crtc->fb.

With async modesetting on its way, this won't actually hold true for
long. Tracking the fb separately, rather than pulling it back out of
the CRTC, is the right thing to do here.

Basically, if you ever want to check crtc->primary->fb, and you're not
in atomic_check, you're doing it wrong.

Cheers,
Daniel