We have some code duplication related to EDID duplication. Add a helper.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 12 ++++++++++++ include/drm/drm_crtc.h | 1 + 2 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c24af1d..412b80f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1264,6 +1264,18 @@ struct edid *drm_get_edid(struct drm_connector *connector, } EXPORT_SYMBOL(drm_get_edid);
+/** + * drm_edid_duplicate - duplicate an EDID and the extensions + * @edid: EDID to duplicate + * + * Return duplicate edid or NULL on allocation failure. + */ +struct edid *drm_edid_duplicate(const struct edid *edid) +{ + return kmemdup(edid, (edid->extensions + 1) * EDID_LENGTH, GFP_KERNEL); +} +EXPORT_SYMBOL(drm_edid_duplicate); + /*** EDID parsing ***/
/** diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b2d08ca..8ab633a 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -980,6 +980,7 @@ extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_m extern bool drm_probe_ddc(struct i2c_adapter *adapter); extern struct edid *drm_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter); +extern struct edid *drm_edid_duplicate(const struct edid *edid); extern int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid); extern void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode); extern void drm_mode_copy(struct drm_display_mode *dst, const struct drm_display_mode *src);
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 95a3159..aed9d67 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2920,18 +2920,12 @@ intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) /* use cached edid if we have one */ if (intel_connector->edid) { struct edid *edid; - int size;
/* invalid edid */ if (IS_ERR(intel_connector->edid)) return NULL;
- size = (intel_connector->edid->extensions + 1) * EDID_LENGTH; - edid = kmemdup(intel_connector->edid, size, GFP_KERNEL); - if (!edid) - return NULL; - - return edid; + return drm_edid_duplicate(edid); }
return drm_get_edid(connector, adapter);
Did you compile or boot this? I get a warning since you are using edid uninitialised, I guess you meant to duplicate intel_connector->edid.
Dave.
drivers/gpu/drm/i915/intel_dp.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 95a3159..aed9d67 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2920,18 +2920,12 @@ intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) /* use cached edid if we have one */ if (intel_connector->edid) { struct edid *edid;
int size; /* invalid edid */ if (IS_ERR(intel_connector->edid)) return NULL;
size = (intel_connector->edid->extensions + 1) * EDID_LENGTH;
edid = kmemdup(intel_connector->edid, size, GFP_KERNEL);
if (!edid)
return NULL;
return edid;
return drm_edid_duplicate(edid); } return drm_get_edid(connector, adapter);
-- 1.7.9.5
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 01 Oct 2013, Dave Airlie airlied@gmail.com wrote:
Did you compile or boot this? I get a warning since you are using edid uninitialised, I guess you meant to duplicate intel_connector->edid.
Hi Dave, quite embarrassing, I thought I did, obviously didn't. Updated patch follows.
BR, Jani.
Dave.
drivers/gpu/drm/i915/intel_dp.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 95a3159..aed9d67 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2920,18 +2920,12 @@ intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) /* use cached edid if we have one */ if (intel_connector->edid) { struct edid *edid;
int size; /* invalid edid */ if (IS_ERR(intel_connector->edid)) return NULL;
size = (intel_connector->edid->extensions + 1) * EDID_LENGTH;
edid = kmemdup(intel_connector->edid, size, GFP_KERNEL);
if (!edid)
return NULL;
return edid;
return drm_edid_duplicate(edid); } return drm_get_edid(connector, adapter);
-- 1.7.9.5
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
v2: duplicate intel_connector->edid, not uninitialized edid (Dave Airlie).
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5614365..6030394 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2969,19 +2969,11 @@ intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
/* use cached edid if we have one */ if (intel_connector->edid) { - struct edid *edid; - int size; - /* invalid edid */ if (IS_ERR(intel_connector->edid)) return NULL;
- size = (intel_connector->edid->extensions + 1) * EDID_LENGTH; - edid = kmemdup(intel_connector->edid, size, GFP_KERNEL); - if (!edid) - return NULL; - - return edid; + return drm_edid_duplicate(intel_connector->edid); }
return drm_get_edid(connector, adapter);
On Tue, Oct 1, 2013 at 5:38 PM, Jani Nikula jani.nikula@intel.com wrote:
v2: duplicate intel_connector->edid, not uninitialized edid (Dave Airlie).
Okay I merged this one,
Thanks, Dave.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c index 4400330..26e089f 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c @@ -101,7 +101,6 @@ static struct edid *vidi_get_edid(struct device *dev, { struct vidi_context *ctx = get_vidi_context(dev); struct edid *edid; - int edid_len;
/* * the edid data comes from user side and it would be set @@ -112,8 +111,7 @@ static struct edid *vidi_get_edid(struct device *dev, return ERR_PTR(-EFAULT); }
- edid_len = (1 + ctx->raw_edid->extensions) * EDID_LENGTH; - edid = kmemdup(ctx->raw_edid, edid_len, GFP_KERNEL); + edid = drm_edid_duplicate(ctx->raw_edid); if (!edid) { DRM_DEBUG_KMS("failed to allocate edid\n"); return ERR_PTR(-ENOMEM); @@ -485,7 +483,6 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void *data, struct exynos_drm_manager *manager; struct exynos_drm_display_ops *display_ops; struct drm_exynos_vidi_connection *vidi = data; - int edid_len;
if (!vidi) { DRM_DEBUG_KMS("user data for vidi is null.\n"); @@ -524,8 +521,7 @@ int vidi_connection_ioctl(struct drm_device *drm_dev, void *data, DRM_DEBUG_KMS("edid data is invalid.\n"); return -EINVAL; } - edid_len = (1 + raw_edid->extensions) * EDID_LENGTH; - ctx->raw_edid = kmemdup(raw_edid, edid_len, GFP_KERNEL); + ctx->raw_edid = drm_edid_duplicate(raw_edid); if (!ctx->raw_edid) { DRM_DEBUG_KMS("failed to allocate raw_edid.\n"); return -ENOMEM;
On Fri, Sep 27, 2013 at 03:08:27PM +0300, Jani Nikula wrote:
We have some code duplication related to EDID duplication. Add a helper.
Lgtm, debated the merits of assuming GFP_KERNEL and inlining, then thought better of it.
All 3, Reviewed-by: Chris Wilson chris@chris-wilson.co.uk -Chris
dri-devel@lists.freedesktop.org