Re: [PATCH] drm/exynos: fixed a issue that plane isn't disabled when released

From: InKi Dae <inki.dae@samsung.com>
To: Paul Menzel <paulepanter@users.sourceforge.net>
Cc: kyungmin.park@samsung.com, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/exynos: fixed a issue that plane isn't disabled when released
Date: Sat, 25 Aug 2012 01:49:49 +0900	[thread overview]
Message-ID: <CAAQKjZPxB=c6XD4c_8=yhekbW8eMvCBOpmsQdMWfzdsLB9EqjA@mail.gmail.com> (raw)
In-Reply-To: <1345803083.14375.10.camel@mattotaupa>

2012/8/24 Paul Menzel <paulepanter@users.sourceforge.net>:
> Dear Inki,
>
>
> Am Freitag, den 24.08.2012, 18:27 +0900 schrieb Inki Dae:
>
> You can shorten the commit summary by leaving out the word issue, which
> is redundant. Maybe one of the following? I do not understand the
> process of releasing so it might be wrong.
>
>         drm/exynos: Disable plane after release
>         drm/exynos: Disable plane when being released
>         drm/exynos: Fix disabling of plane when released
>
>> when drm is released, drm framebuffers are released and all crtcs using
>> same framebuffer and also all gem buffers used but plane isn't disabled
>> so when crtc and encoder are turned on, overlay can access to invalid memory
>> because plane still has memory address released already.
>> this patch makes sure that each plane is disabled when released.
>
> Please use sentences to make it easier to understand.
>
> Also describe the solution. Maybe something like this.
>
>         This patch ensures that each plane is disabled when released, by
>         adding a new function and adding that to the helper functions.
>
> Is there a report for that issue? Did you experience it in your setup?
> Is this tested?
>

Thank you for your advice. acutally, I missed some sentences for
describing this patch so I will send it again.

Thanks,
Inki Dae

>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_encoder.c |   15 +++++++++++++++
>>  1 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> index 92f9acf..96a10c3 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> @@ -214,12 +214,27 @@ static void exynos_drm_encoder_commit(struct drm_encoder *encoder)
>>               manager_ops->commit(manager->dev);
>>  }
>>
>> +static void exynos_drm_encoder_disable(struct drm_encoder *encoder)
>> +{
>> +     struct drm_plane *plane;
>> +     struct drm_device *dev = encoder->dev;
>> +
>> +     exynos_drm_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
>> +
>> +     /* all planes connected to this encoder should be also disabled. */
>> +     list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
>> +             if (plane->crtc == encoder->crtc)
>> +                     plane->funcs->disable_plane(plane);
>> +     }
>> +}
>> +
>>  static struct drm_encoder_helper_funcs exynos_encoder_helper_funcs = {
>>       .dpms           = exynos_drm_encoder_dpms,
>>       .mode_fixup     = exynos_drm_encoder_mode_fixup,
>>       .mode_set       = exynos_drm_encoder_mode_set,
>>       .prepare        = exynos_drm_encoder_prepare,
>>       .commit         = exynos_drm_encoder_commit,
>> +     .disable        = exynos_drm_encoder_disable,
>>  };
>>
>>  static void exynos_drm_encoder_destroy(struct drm_encoder *encoder)
>
>
> Thanks,
>
> Paul
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>