[PATCH 0/3] drm/exynos: modify usage of wait for vblank

[Inki Dae]

2012/12/5 Inki Dae <inki.dae at samsung.com>

>
>
> 2012/12/5 Prathyush K <prathyush.k at samsung.com>
>
>> This patchset fixes a few issues with use of wait for vblank in
>> exynos drm.
>>
>> Please apply these three patches without "drm/exynos: release fb pended
>> by page flip"
>> patch.
>>
>> Patch 1: modify wait for vsync functions to use wait queues
>> This modifies the wait_for_vblank functions to use wait queues
>> thus ensuring that the current task goes to sleep without taking
>> up CPU while waiting.
>>
>> Patch 2: move wait_for_vblank to manager_ops
>> Currently, we do not call wait for vblank if encoder is in DPMS_OFF
>> state inside exynos_drm_encoder_complete_scanout function. This is
>> because wait for vblank is treated as an overlay op.
>
> This can be
>> modified to a manager_op thus removing the above check for DPMS_OFF.
>> This fixes a crash while removing a fb.
>> While removing the current fb (crtc->fb == fb) the encoder
>> dpms off is called and then the framebuffer is removed. So in
>> this case, the wait for vblank is not called thus leading to a crash
>> (since fb might get removed before dma is finished)
>>
>> Patch 3: do not disable crtc if already off
>> We should not disable the overlay if the crtc is in DPMS_OFF state.
>> Also, the disable function should not call DPMS_OFF of the crtc.
>> This is required to ensure we are able to wait for vblank
>> before freeing any framebuffers after disabling the crtc.
>>
>> With these three patches and without "drm/exynos: release fb pended by
>> page flip"
>> I could not find any crash/page_fault in drm with fimd/hdmi during hotplug
>> and page flips.
>>
>>
> It seems better than old one and also including some exception codes,
> Patch 3 we did't consider. Ok, we will test these patches and merge them
> instead of old one if no problem.
>
>

Tested and worked fine. But with this patch and without "drm/exynos:
release fb pended by page flip", we would still have potential issue to
page fault like below,
1. dma is accessing no current fb
2. the fb to be released is not current fb so the fb would be released
without disabling crtc when drm is released.
3. but the memory region to no current fb is being accessed by the dma so
the page fault would be induced. This is a rare case but possible issue.

Anyway I removed the patch, "drm/exynos: release fb pended by page flip".
And I hope this issue can be resolved by Daniel's fixup later.

And I think it's better to separate your patch set into 4 patches like
below,
1. the patch including only exynos drm framework part.
2. the patch including only fimd driver part.
3. the patch including only hdmi driver part.
4. the patch including exception code.(previous Patch 3)

Please, re-send them.

Thanks,
Inki Dae

Thanks,
> Inki Dae
>
>
>> Prathyush K (3)
>>   drm/exynos: modify wait for vsync functions to use wait queues
>>   drm/exynos: move wait_for_vblank to manager_ops
>>   drm/exynos: do not disable crtc if already off
>>
>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c    |    6 +++-
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h     |   20 ++------------
>>  drivers/gpu/drm/exynos/exynos_drm_encoder.c |   15 +++--------
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c    |   37
>> ++++++++++++++++++--------
>>  drivers/gpu/drm/exynos/exynos_drm_hdmi.c    |   22 ++++++++--------
>>  drivers/gpu/drm/exynos/exynos_drm_hdmi.h    |    2 +-
>>  drivers/gpu/drm/exynos/exynos_mixer.c       |   37
>> +++++++++++++++++---------
>>  7 files changed, 73 insertions(+), 66 deletions(-)
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121205/e1b4b67f/attachment.html>