* [PATCH v4] drm/exynos: let drm handle edid allocations
@ 2013-01-04 12:59 Rahul Sharma
2013-01-10 6:17 ` Inki Dae
0 siblings, 1 reply; 2+ messages in thread
From: Rahul Sharma @ 2013-01-04 12:59 UTC (permalink / raw)
To: dri-devel; +Cc: kgene.kim, seanpaul, sw0312.kim, joshi
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.
v4:
- removed error check for drm_mode_connector_update_edid_property
which is expected to fail for Virtual Connectors like VIDI.
Thanks to Seung-Woo Kim.
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 | 33 ++++++++++++---------------
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, 46 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..4c5b685 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;
+ struct edid *edid = NULL;
+ unsigned int count = 0;
+ int ret;
DRM_DEBUG_KMS("%s\n", __FILE__);
@@ -114,27 +115,21 @@ 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;
+ count = drm_add_edid_modes(connector, edid);
+ if (count < 0) {
+ DRM_ERROR("Add edid modes failed %d\n", count);
+ goto out;
}
drm_mode_connector_update_edid_property(connector, edid);
- count = drm_add_edid_modes(connector, edid);
- kfree(edid);
} else {
struct exynos_drm_panel_info *panel;
struct drm_display_mode *mode = drm_mode_create(connector->dev);
@@ -161,6 +156,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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v4] drm/exynos: let drm handle edid allocations
2013-01-04 12:59 [PATCH v4] drm/exynos: let drm handle edid allocations Rahul Sharma
@ 2013-01-10 6:17 ` Inki Dae
0 siblings, 0 replies; 2+ messages in thread
From: Inki Dae @ 2013-01-10 6:17 UTC (permalink / raw)
To: Rahul Sharma; +Cc: kgene.kim, sw0312.kim, seanpaul, dri-devel, joshi
Applied.
Thanks,
Inki Dae
2013/1/4 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.
>
> v4:
> - removed error check for drm_mode_connector_update_edid_property
> which is expected to fail for Virtual Connectors like VIDI.
> Thanks to Seung-Woo Kim.
>
> 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 | 33 ++++++++++++---------------
> 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, 46 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..4c5b685 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;
> + struct edid *edid = NULL;
> + unsigned int count = 0;
> + int ret;
>
> DRM_DEBUG_KMS("%s\n", __FILE__);
>
> @@ -114,27 +115,21 @@ 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;
> + count = drm_add_edid_modes(connector, edid);
> + if (count < 0) {
> + DRM_ERROR("Add edid modes failed %d\n", count);
> + goto out;
> }
>
> drm_mode_connector_update_edid_property(connector, edid);
> - count = drm_add_edid_modes(connector, edid);
> - kfree(edid);
> } else {
> struct exynos_drm_panel_info *panel;
> struct drm_display_mode *mode = drm_mode_create(connector->dev);
> @@ -161,6 +156,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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-01-10 6:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-04 12:59 [PATCH v4] drm/exynos: let drm handle edid allocations Rahul Sharma
2013-01-10 6:17 ` Inki Dae
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).