[RFCv3 04/14] drm/exynos: Restrict plane loops to only operate on overlay planes

[Daniel Kurtz]

On Wed, Mar 19, 2014 at 7:51 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Tue, Mar 18, 2014 at 05:22:49PM -0700, Matt Roper wrote:
>> Before we add additional types of planes to the DRM plane list, ensure
>> that existing loops over all planes continue to operate only on
>> "overlay" planes and ignore primary & cursor planes.
>>
>> Cc: Inki Dae <inki.dae at samsung.com>
>> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_encoder.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> index 06f1b2a..2fa2685 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> @@ -127,6 +127,9 @@ static void disable_plane_to_crtc(struct drm_device *dev,
>>        * (encoder->crtc)
>>        */
>>       list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
>> +             if (plane->type != DRM_PLANE_TYPE_OVERLAY)
>
> I think a drm_for_each_legacy_plane iteration helper would be neat for
> this one and the following i915 patch.
> -Daniel
>
>> +                     continue;
>> +
>>               if (plane->crtc == old_crtc) {
>>                       /*
>>                        * do not change below call order.
>> @@ -247,6 +250,9 @@ static void exynos_drm_encoder_disable(struct drm_encoder *encoder)
>>
>>       /* all planes connected to this encoder should be also disabled. */
>>       list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
>> +             if (plane->type != DRM_PLANE_TYPE_OVERLAY)
>> +                     continue;
>> +
>>               if (plane->crtc == encoder->crtc)
>>                       plane->funcs->disable_plane(plane);
>>       }

The original loop disables all planes attached to a crtc when
disabling an encoder attached to the same crtc.  It was added by:

commit bcf4cef94294992f7cd11d5a90fa58b0eae6c795
Author: Inki Dae <inki.dae at samsung.com>
Date:   Fri Aug 24 10:54:12 2012 -0700

    drm/exynos: Disable plane when released

    this patch ensures that each plane connected to encoder is disabled
    when released, by adding disable callback function of encoder helper

    we had faced with one issue that invalid memory is accessed by dma
    once drm is released and then the dma is turned on again. actually,
    in our case, page fault was incurred with iommu. the reason is that
    a gem buffer accessed by the dma is also released once drm is released.

    so this patch would fix this issue ensuring the dma is disabled
    when released.


An encoder receives and encodes the mixed output of all of the
planes/overlays.  It would seem that whether the individual planes
themselves are enabled or not should be completely independent of the
status any encoder.  However, I find the code in exynos_drm_encoder.c
very difficult to follow, so perhaps there is some extra linkage
between encoder/crtc/plane that is exynos specific.

In any case, judging from the commit message, this disable loop should
probably still iterate over all of the planes, not just the
"DRM_PLANE_TYPE_OVERLAY" ones.  So, I think this new patch is
incorrect.


>> --
>> 1.8.5.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel