dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/edid: add drm_edid_duplicate
@ 2013-09-27 12:08 Jani Nikula
  2013-09-27 12:08 ` [PATCH 2/3] drm/i915/dp: use drm_edid_duplicate Jani Nikula
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Jani Nikula @ 2013-09-27 12:08 UTC (permalink / raw)
  To: dri-devel, intel-gfx

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);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] drm/i915/dp: use drm_edid_duplicate
  2013-09-27 12:08 [PATCH 1/3] drm/edid: add drm_edid_duplicate Jani Nikula
@ 2013-09-27 12:08 ` Jani Nikula
  2013-10-01  0:33   ` Dave Airlie
  2013-09-27 12:08 ` [PATCH 3/3] drm/exynos: " Jani Nikula
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2013-09-27 12:08 UTC (permalink / raw)
  To: dri-devel, intel-gfx

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);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] drm/exynos: use drm_edid_duplicate
  2013-09-27 12:08 [PATCH 1/3] drm/edid: add drm_edid_duplicate Jani Nikula
  2013-09-27 12:08 ` [PATCH 2/3] drm/i915/dp: use drm_edid_duplicate Jani Nikula
@ 2013-09-27 12:08 ` Jani Nikula
  2013-09-27 12:24 ` [PATCH 1/3] drm/edid: add drm_edid_duplicate Chris Wilson
  2013-09-27 12:42 ` Damien Lespiau
  3 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2013-09-27 12:08 UTC (permalink / raw)
  To: dri-devel, intel-gfx

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;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] drm/edid: add drm_edid_duplicate
  2013-09-27 12:08 [PATCH 1/3] drm/edid: add drm_edid_duplicate Jani Nikula
  2013-09-27 12:08 ` [PATCH 2/3] drm/i915/dp: use drm_edid_duplicate Jani Nikula
  2013-09-27 12:08 ` [PATCH 3/3] drm/exynos: " Jani Nikula
@ 2013-09-27 12:24 ` Chris Wilson
  2013-09-27 12:42 ` Damien Lespiau
  3 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2013-09-27 12:24 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, dri-devel

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

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] drm/edid: add drm_edid_duplicate
  2013-09-27 12:08 [PATCH 1/3] drm/edid: add drm_edid_duplicate Jani Nikula
                   ` (2 preceding siblings ...)
  2013-09-27 12:24 ` [PATCH 1/3] drm/edid: add drm_edid_duplicate Chris Wilson
@ 2013-09-27 12:42 ` Damien Lespiau
  3 siblings, 0 replies; 9+ messages in thread
From: Damien Lespiau @ 2013-09-27 12:42 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, dri-devel

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.

git grep kmemdup.*edid drivers/gpu/drm/ also returns 3 hits in nouveau
here, should anyone be bored.

-- 
Damien

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] drm/i915/dp: use drm_edid_duplicate
  2013-09-27 12:08 ` [PATCH 2/3] drm/i915/dp: use drm_edid_duplicate Jani Nikula
@ 2013-10-01  0:33   ` Dave Airlie
  2013-10-01  7:37     ` [Intel-gfx] " Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Airlie @ 2013-10-01  0:33 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx@lists.freedesktop.org, dri-devel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/dp: use drm_edid_duplicate
  2013-10-01  0:33   ` Dave Airlie
@ 2013-10-01  7:37     ` Jani Nikula
  2013-10-01  7:38       ` [PATCH] " Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2013-10-01  7:37 UTC (permalink / raw)
  To: Dave Airlie; +Cc: intel-gfx@lists.freedesktop.org, dri-devel

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

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] drm/i915/dp: use drm_edid_duplicate
  2013-10-01  7:37     ` [Intel-gfx] " Jani Nikula
@ 2013-10-01  7:38       ` Jani Nikula
  2013-10-09  5:30         ` Dave Airlie
  0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2013-10-01  7:38 UTC (permalink / raw)
  To: Dave Airlie; +Cc: jani.nikula, intel-gfx, dri-devel

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);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915/dp: use drm_edid_duplicate
  2013-10-01  7:38       ` [PATCH] " Jani Nikula
@ 2013-10-09  5:30         ` Dave Airlie
  0 siblings, 0 replies; 9+ messages in thread
From: Dave Airlie @ 2013-10-09  5:30 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx@lists.freedesktop.org, dri-devel

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-10-09  5:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-27 12:08 [PATCH 1/3] drm/edid: add drm_edid_duplicate Jani Nikula
2013-09-27 12:08 ` [PATCH 2/3] drm/i915/dp: use drm_edid_duplicate Jani Nikula
2013-10-01  0:33   ` Dave Airlie
2013-10-01  7:37     ` [Intel-gfx] " Jani Nikula
2013-10-01  7:38       ` [PATCH] " Jani Nikula
2013-10-09  5:30         ` Dave Airlie
2013-09-27 12:08 ` [PATCH 3/3] drm/exynos: " Jani Nikula
2013-09-27 12:24 ` [PATCH 1/3] drm/edid: add drm_edid_duplicate Chris Wilson
2013-09-27 12:42 ` Damien Lespiau

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).