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

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.

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 | 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_drm_vidi.c | 19 +++++++++----- drivers/gpu/drm/exynos/exynos_hdmi.c | 25 ++++++++----------- 6 files changed, 50 insertions(+), 47 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);
  

MAX_EDID definition isn't used anymore. Please remove it.

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