| Message ID | 1357128090-3781-1-git-send-email-rahul.sharma@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Rahul, With this patch, I got a error like below, # echo 1 > /sys/devices/vidi.5/connection [ 149.485000] [drm:exynos_drm_connector_get_modes] *ERROR* update edid property failed(-22) There is something wrong. Please check it out again. 2013/1/2 Rahul Sharma <rahul.sharma@samsung.com>: > 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. > > v3: > - removed MAX_EDID as it is not used anymore. > > v2: > - changed vidi_get_edid callback inside vidi driver. > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com> > Signed-off-by: Seung-Woo Kim <sw0312.kim@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 | 37 ++++++++++++++------------- > 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_drm_vidi.c | 19 +++++++++----- > drivers/gpu/drm/exynos/exynos_hdmi.c | 25 ++++++++---------- > 6 files changed, 50 insertions(+), 48 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c > index ab37437..221ec72 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_connector.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c > @@ -18,7 +18,6 @@ > #include "exynos_drm_drv.h" > #include "exynos_drm_encoder.h" > > -#define MAX_EDID 256 > #define to_exynos_connector(x) container_of(x, struct exynos_drm_connector,\ > drm_connector) > > @@ -96,7 +95,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 +115,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 +160,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); > diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c > index 55793c4..a17d752 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c > @@ -108,18 +108,17 @@ static bool drm_hdmi_is_connected(struct device *dev) > return false; > } > > -static int drm_hdmi_get_edid(struct device *dev, > - struct drm_connector *connector, u8 *edid, int len) > +struct edid *drm_hdmi_get_edid(struct device *dev, > + struct drm_connector *connector) > { > struct drm_hdmi_context *ctx = to_context(dev); > > DRM_DEBUG_KMS("%s\n", __FILE__); > > if (hdmi_ops && hdmi_ops->get_edid) > - return hdmi_ops->get_edid(ctx->hdmi_ctx->ctx, connector, edid, > - len); > + return hdmi_ops->get_edid(ctx->hdmi_ctx->ctx, connector); > > - return 0; > + return NULL; > } > > static int drm_hdmi_check_timing(struct device *dev, void *timing) > diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h > index 784a7e9..d80516f 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h > +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h > @@ -30,8 +30,8 @@ struct exynos_drm_hdmi_context { > struct exynos_hdmi_ops { > /* display */ > bool (*is_connected)(void *ctx); > - int (*get_edid)(void *ctx, struct drm_connector *connector, > - u8 *edid, int len); > + struct edid *(*get_edid)(void *ctx, > + struct drm_connector *connector); > int (*check_timing)(void *ctx, void *timing); > int (*power_on)(void *ctx, int mode); > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c > index 99bfc38..3260035 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c > @@ -98,10 +98,12 @@ static bool vidi_display_is_connected(struct device *dev) > return ctx->connected ? true : false; > } > > -static int vidi_get_edid(struct device *dev, struct drm_connector *connector, > - u8 *edid, int len) > +static struct edid *vidi_get_edid(struct device *dev, > + struct drm_connector *connector) > { > struct vidi_context *ctx = get_vidi_context(dev); > + struct edid *edid; > + int edid_len; > > DRM_DEBUG_KMS("%s\n", __FILE__); > > @@ -111,13 +113,18 @@ static int vidi_get_edid(struct device *dev, struct drm_connector *connector, > */ > if (!ctx->raw_edid) { > DRM_DEBUG_KMS("raw_edid is null.\n"); > - return -EFAULT; > + return ERR_PTR(-EFAULT); > } > > - memcpy(edid, ctx->raw_edid, min((1 + ctx->raw_edid->extensions) > - * EDID_LENGTH, len)); > + edid_len = (1 + ctx->raw_edid->extensions) * EDID_LENGTH; > + edid = kzalloc(edid_len, GFP_KERNEL); > + if (!edid) { > + DRM_DEBUG_KMS("failed to allocate edid\n"); > + return ERR_PTR(-ENOMEM); > + } > > - return 0; > + memcpy(edid, ctx->raw_edid, edid_len); > + return edid; > } > > static void *vidi_get_panel(struct device *dev) > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c > index 2c46b6c..36e9214 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -1391,8 +1391,7 @@ static bool hdmi_is_connected(void *ctx) > return hdata->hpd; > } > > -static int hdmi_get_edid(void *ctx, struct drm_connector *connector, > - u8 *edid, int len) > +static struct edid *hdmi_get_edid(void *ctx, struct drm_connector *connector) > { > struct edid *raw_edid; > struct hdmi_context *hdata = ctx; > @@ -1400,22 +1399,18 @@ static int hdmi_get_edid(void *ctx, struct drm_connector *connector, > DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); > > if (!hdata->ddc_port) > - return -ENODEV; > + return ERR_PTR(-ENODEV); > > raw_edid = drm_get_edid(connector, hdata->ddc_port->adapter); > - if (raw_edid) { > - hdata->dvi_mode = !drm_detect_hdmi_monitor(raw_edid); > - memcpy(edid, raw_edid, min((1 + raw_edid->extensions) > - * EDID_LENGTH, len)); > - DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n", > - (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), > - raw_edid->width_cm, raw_edid->height_cm); > - kfree(raw_edid); > - } else { > - return -ENODEV; > - } > + if (!raw_edid) > + return ERR_PTR(-ENODEV); > > - return 0; > + hdata->dvi_mode = !drm_detect_hdmi_monitor(raw_edid); > + DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n", > + (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), > + raw_edid->width_cm, raw_edid->height_cm); > + > + return raw_edid; > } > > static int hdmi_v13_check_timing(struct fb_videomode *check_timing) > -- > 1.8.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c index ab37437..221ec72 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_connector.c +++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c @@ -18,7 +18,6 @@ #include "exynos_drm_drv.h" #include "exynos_drm_encoder.h" -#define MAX_EDID 256 #define to_exynos_connector(x) container_of(x, struct exynos_drm_connector,\ drm_connector) @@ -96,7 +95,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 +115,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 +160,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); diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c index 55793c4..a17d752 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c @@ -108,18 +108,17 @@ static bool drm_hdmi_is_connected(struct device *dev) return false; } -static int drm_hdmi_get_edid(struct device *dev, - struct drm_connector *connector, u8 *edid, int len) +struct edid *drm_hdmi_get_edid(struct device *dev, + struct drm_connector *connector) { struct drm_hdmi_context *ctx = to_context(dev); DRM_DEBUG_KMS("%s\n", __FILE__); if (hdmi_ops && hdmi_ops->get_edid) - return hdmi_ops->get_edid(ctx->hdmi_ctx->ctx, connector, edid, - len); + return hdmi_ops->get_edid(ctx->hdmi_ctx->ctx, connector); - return 0; + return NULL; } static int drm_hdmi_check_timing(struct device *dev, void *timing) diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h index 784a7e9..d80516f 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h @@ -30,8 +30,8 @@ struct exynos_drm_hdmi_context { struct exynos_hdmi_ops { /* display */ bool (*is_connected)(void *ctx); - int (*get_edid)(void *ctx, struct drm_connector *connector, - u8 *edid, int len); + struct edid *(*get_edid)(void *ctx, + struct drm_connector *connector); int (*check_timing)(void *ctx, void *timing); int (*power_on)(void *ctx, int mode); diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c index 99bfc38..3260035 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c @@ -98,10 +98,12 @@ static bool vidi_display_is_connected(struct device *dev) return ctx->connected ? true : false; } -static int vidi_get_edid(struct device *dev, struct drm_connector *connector, - u8 *edid, int len) +static struct edid *vidi_get_edid(struct device *dev, + struct drm_connector *connector) { struct vidi_context *ctx = get_vidi_context(dev); + struct edid *edid; + int edid_len; DRM_DEBUG_KMS("%s\n", __FILE__); @@ -111,13 +113,18 @@ static int vidi_get_edid(struct device *dev, struct drm_connector *connector, */ if (!ctx->raw_edid) { DRM_DEBUG_KMS("raw_edid is null.\n"); - return -EFAULT; + return ERR_PTR(-EFAULT); } - memcpy(edid, ctx->raw_edid, min((1 + ctx->raw_edid->extensions) - * EDID_LENGTH, len)); + edid_len = (1 + ctx->raw_edid->extensions) * EDID_LENGTH; + edid = kzalloc(edid_len, GFP_KERNEL); + if (!edid) { + DRM_DEBUG_KMS("failed to allocate edid\n"); + return ERR_PTR(-ENOMEM); + } - return 0; + memcpy(edid, ctx->raw_edid, edid_len); + return edid; } static void *vidi_get_panel(struct device *dev) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 2c46b6c..36e9214 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -1391,8 +1391,7 @@ static bool hdmi_is_connected(void *ctx) return hdata->hpd; } -static int hdmi_get_edid(void *ctx, struct drm_connector *connector, - u8 *edid, int len) +static struct edid *hdmi_get_edid(void *ctx, struct drm_connector *connector) { struct edid *raw_edid; struct hdmi_context *hdata = ctx; @@ -1400,22 +1399,18 @@ static int hdmi_get_edid(void *ctx, struct drm_connector *connector, DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); if (!hdata->ddc_port) - return -ENODEV; + return ERR_PTR(-ENODEV); raw_edid = drm_get_edid(connector, hdata->ddc_port->adapter); - if (raw_edid) { - hdata->dvi_mode = !drm_detect_hdmi_monitor(raw_edid); - memcpy(edid, raw_edid, min((1 + raw_edid->extensions) - * EDID_LENGTH, len)); - DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n", - (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), - raw_edid->width_cm, raw_edid->height_cm); - kfree(raw_edid); - } else { - return -ENODEV; - } + if (!raw_edid) + return ERR_PTR(-ENODEV); - return 0; + hdata->dvi_mode = !drm_detect_hdmi_monitor(raw_edid); + DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n", + (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), + raw_edid->width_cm, raw_edid->height_cm); + + return raw_edid; } static int hdmi_v13_check_timing(struct fb_videomode *check_timing)