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 > > 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>
