[PATCH] drm/exynos: let drm handle edid allocations

[Rahul Sharma]

On Wed, Jan 2, 2013 at 6:05 AM, 김승우 <sw0312.kim at samsung.com> wrote:
> Hi Rahul,
>
> On 2012년 12월 28일 16:01, Rahul Sharma wrote:
>> There's no need to allocate edid twice and do a memcpy when drm helpers
>> exist to do just that. This patch cleans that interaction up, and
>> doesn't keep the edid hanging around in the connector.
>
> Basically, I agree about this idea. But exynos_drm_vidi also uses
> display_ops->get_edid(), so vidi should be considered.
>
> Best Regards,
> - Seung-Woo Kim

Thanks Seung Woo,

I agree to it. I should have added the changes to vidi driver also. I
will refer the
patches you provided for reference (sent off-line) and post v2.

regards,
Rahul Sharma.

>
>>
>> Signed-off-by: Sean Paul <seanpaul at chromium.org>
>> Signed-off-by: Rahul Sharma <rahul.sharma at samsung.com>
>> ---
>> This patch is based on branch "exynos-drm-next" at
>> http://git.kernel.org/?p=linux/kernel/git/daeinki/drm-exynos.git
>>
>>  drivers/gpu/drm/exynos/exynos_drm_connector.c | 36 ++++++++++++++-------------
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h       |  4 +--
>>  drivers/gpu/drm/exynos/exynos_drm_hdmi.c      |  9 +++----
>>  drivers/gpu/drm/exynos/exynos_drm_hdmi.h      |  4 +--
>>  drivers/gpu/drm/exynos/exynos_hdmi.c          | 25 ++++++++-----------
>>  5 files changed, 37 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c
>> index ab37437..7ee43aa 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
>> @@ -96,7 +96,9 @@ static int exynos_drm_connector_get_modes(struct drm_connector *connector)
>>                                       to_exynos_connector(connector);
>>       struct exynos_drm_manager *manager = exynos_connector->manager;
>>       struct exynos_drm_display_ops *display_ops = manager->display_ops;
>> -     unsigned int count;
>> +     unsigned int count = 0;
>> +     struct edid *edid = NULL;
>> +     int ret;
>>
>>       DRM_DEBUG_KMS("%s\n", __FILE__);
>>
>> @@ -114,27 +116,25 @@ static int exynos_drm_connector_get_modes(struct drm_connector *connector)
>>        * because lcd panel has only one mode.
>>        */
>>       if (display_ops->get_edid) {
>> -             int ret;
>> -             void *edid;
>> -
>> -             edid = kzalloc(MAX_EDID, GFP_KERNEL);
>> -             if (!edid) {
>> -                     DRM_ERROR("failed to allocate edid\n");
>> -                     return 0;
>> +             edid = display_ops->get_edid(manager->dev, connector);
>> +             if (IS_ERR_OR_NULL(edid)) {
>> +                     ret = PTR_ERR(edid);
>> +                     edid = NULL;
>> +                     DRM_ERROR("Panel operation get_edid failed %d\n", ret);
>> +                     goto out;
>>               }
>>
>> -             ret = display_ops->get_edid(manager->dev, connector,
>> -                                             edid, MAX_EDID);
>> -             if (ret < 0) {
>> -                     DRM_ERROR("failed to get edid data.\n");
>> -                     kfree(edid);
>> -                     edid = NULL;
>> -                     return 0;
>> +             ret = drm_mode_connector_update_edid_property(connector, edid);
>> +             if (ret) {
>> +                     DRM_ERROR("update edid property failed(%d)\n", ret);
>> +                     goto out;
>>               }
>>
>> -             drm_mode_connector_update_edid_property(connector, edid);
>>               count = drm_add_edid_modes(connector, edid);
>> -             kfree(edid);
>> +             if (count < 0) {
>> +                     DRM_ERROR("Add edid modes failed %d\n", count);
>> +                     goto out;
>> +             }
>>       } else {
>>               struct exynos_drm_panel_info *panel;
>>               struct drm_display_mode *mode = drm_mode_create(connector->dev);
>> @@ -161,6 +161,8 @@ static int exynos_drm_connector_get_modes(struct drm_connector *connector)
>>               count = 1;
>>       }
>>
>> +out:
>> +     kfree(edid);
>>       return count;
>>  }
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index b9e51bc..4606fac 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -148,8 +148,8 @@ struct exynos_drm_overlay {
>>  struct exynos_drm_display_ops {
>>       enum exynos_drm_output_type type;
>>       bool (*is_connected)(struct device *dev);
>> -     int (*get_edid)(struct device *dev, struct drm_connector *connector,
>> -                             u8 *edid, int len);
>> +     struct edid *(*get_edid)(struct device *dev,
>> +                     struct drm_connector *connector);
>>       void *(*get_panel)(struct device *dev);
>>       int (*check_timing)(struct device *dev, void *timing);
>>       int (*power_on)(struct device *dev, int mode);
>
> <snip>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel