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.
Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Rahul Sharma rahul.sharma@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); diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c index 3a8eea6..681a4cb 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 ae4b6ae..45b2ce0 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_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 8cb42ad..708108b 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -980,8 +980,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; @@ -989,22 +988,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)
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@chromium.org Signed-off-by: Rahul Sharma rahul.sharma@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;
count = drm_add_edid_modes(connector, edid);drm_mode_connector_update_edid_property(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,
void *(*get_panel)(struct device *dev); int (*check_timing)(struct device *dev, void *timing); int (*power_on)(struct device *dev, int mode);struct drm_connector *connector);
<snip>
On Wed, Jan 2, 2013 at 6:05 AM, 김승우 sw0312.kim@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@chromium.org Signed-off-by: Rahul Sharma rahul.sharma@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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org