Synchronization between a crtc mode_set and page_flip?

[Rob Clark]

On Fri, Apr 4, 2014 at 3:21 AM, Archit Taneja <archit at ti.com> wrote:
> On Friday 04 April 2014 02:54 AM, Daniel Vetter wrote:
>>
>> On Thu, Apr 03, 2014 at 02:28:32PM +0530, Archit Taneja wrote:
>>>
>>> On Wednesday 02 April 2014 06:41 PM, Rob Clark wrote:
>>>>
>>>> On Wed, Apr 2, 2014 at 5:52 AM, Archit Taneja <archit at ti.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I was trying to figure out how we are supposed to manage
>>>>> synchronization
>>>>> between a mode_set and a page_flip called on a crtc.
>>>>>
>>>>> Say, if a mode_set is immediately followed by a page_flip. The driver
>>>>> can't
>>>>> process the page_flip straight away since the hardware is still
>>>>> completing
>>>>> the mode_set.
>>>>
>>>>
>>>> I guess setcrtc is expected to be synchronous(ish).. so a lot of
>>>> userspace won't expect the first pageflip to fail with -EBUSY.
>>>
>>>
>>> Okay, thanks. I guess having setcrtc synchronous isn't that bad.
>>
>>
>> Yeah, atm I think the rules are that pageflip can only return -EBUSY if
>> there's still a pageflip ongoing. Everything else is presumed to be at
>> least implicitly ordered, so if your hw can do async modesets then you
>> need to synchronize. Also if there's still a pageflip outstanding you need
>> to wait for that to complete in your set_config callback first (usually in
>> the set_base or the crtc->disable/prepare hooks when using the crtc
>> helpers).
>
>
> Thanks for the info. I didn't realize that the prepare/commit hooks get
> executed when using drm_crtc_helper_set_mode() for the set_config helper.
>
> Rob,
>
> We disable the crtc in prepare, and enable it in commit.
>
> If setcrtc changes the mode,  I don't see how apply worker can execute
> prepare() -> mode_set() -> commit() hooks in a row for the crtc
> drm_crtc_helper_set_mode(). We can't queue more than one 'apply' of the same
> entity. So the latter queues are bound to get rejected, I see
> omap_crtc_apply() bailing out for crtc's apply in this case.

What is *supposed* to happen is that whenever apply worker gets a
chance to run, it applies whatever are the most recent settings.   So
even if you somehow managed to do two modesets before the apply worker
had a chance to run, it would simply apply whatever is the most recent
mode..  so modeset is not synchronized, but it is serialized.

So we don't actually need to queue up the same apply multiple times.
Looking at omap_crtc_pre_apply(), I think that should be fine.  Even
if we miss the dpms(OFF) from the prepare step, omap_crtc_pre_apply()
will still disable and reenable power if switching encoder.

That said, we probably do at least need to handle a pending pageflip
during setcrtc.  I think it would be reasonable to make prepare()
block until no pending pageflip or apply.  (Note: a pageflip to a
buffer still being rendered by the GPU won't have triggered an apply
*yet*)

BR,
-R

> I guess making prepare() and mode_set() wait for a vsync should fix this.
> Or, combining mode_set and commit as a single queue to apply should work
> too. Any suggestions?
>
> Thanks,
> Archit
>