[PATCH 1/6] Revert "drm: Use a flexible array member for blob property data"

[PATCH 1/6] Revert "drm: Use a flexible array member for blob property data"

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Using a flexible array for the blob data was a mistake by me. It
forces all users of the blob data to cast blob->data to something
else. void* is clearly superior so let's go back to the original
scheme.

Not a clean revert as the code has moved.

This reverts commit d63f5e6bf6f2a1573ea39c9937cdf5ab0b3a4b77.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_property.c | 1 +
 include/drm/drm_property.h     | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index bae50e6b819d..0f6620fea3de 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -550,6 +550,7 @@ drm_property_create_blob(struct drm_device *dev, size_t length,
 	/* This must be explicitly initialised, so we can safely call list_del
 	 * on it in the removal handler, even if it isn't in a file list. */
 	INIT_LIST_HEAD(&blob->head_file);
+	blob->data = (void *)blob + sizeof(*blob);
 	blob->length = length;
 	blob->dev = dev;
 
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index 8a522b4bed40..265fd1f2e112 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -209,7 +209,7 @@ struct drm_property_blob {
 	struct list_head head_global;
 	struct list_head head_file;
 	size_t length;
-	unsigned char data[];
+	void *data;
 };
 
 struct drm_prop_enum_list {
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/6] drm: Remove now pointelss blob->data casts

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that blob->data is void* again we don't need the casts anymore.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c        | 3 +--
 drivers/gpu/drm/drm_atomic_helper.c | 2 +-
 drivers/gpu/drm/drm_edid.c          | 3 +--
 drivers/gpu/drm/drm_fb_helper.c     | 2 +-
 drivers/gpu/drm/drm_plane.c         | 2 +-
 5 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 46733d534587..8945357212ba 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -391,8 +391,7 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 	if (blob) {
 		if (blob->length != sizeof(struct drm_mode_modeinfo) ||
 		    drm_mode_convert_umode(state->crtc->dev, &state->mode,
-		                           (const struct drm_mode_modeinfo *)
-		                            blob->data))
+					   blob->data))
 			return -EINVAL;
 
 		state->mode_blob = drm_property_blob_get(blob);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ae3cbfe9e01c..6211a1b20405 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3816,7 +3816,7 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 	}
 
 	/* Prepare GAMMA_LUT with the legacy values. */
-	blob_data = (struct drm_color_lut *) blob->data;
+	blob_data = blob->data;
 	for (i = 0; i < size; i++) {
 		blob_data[i].red = red[i];
 		blob_data[i].green = green[i];
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 788fee4b4bf9..134069f36482 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1575,8 +1575,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
 	struct edid *override = NULL;
 
 	if (connector->override_edid)
-		override = drm_edid_duplicate((const struct edid *)
-					      connector->edid_blob_ptr->data);
+		override = drm_edid_duplicate(connector->edid_blob_ptr->data);
 
 	if (!override)
 		override = drm_load_edid_firmware(connector);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 035784ddd133..0646b108030b 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1351,7 +1351,7 @@ static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
 	if (IS_ERR(gamma_lut))
 		return gamma_lut;
 
-	lut = (struct drm_color_lut *)gamma_lut->data;
+	lut = gamma_lut->data;
 	if (cmap->start || cmap->len != size) {
 		u16 *r = crtc->gamma_store;
 		u16 *g = r + crtc->gamma_size;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 09de6ecb3968..9851616cf0f3 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -104,7 +104,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
 	if (IS_ERR(blob))
 		return -1;
 
-	blob_data = (struct drm_format_modifier_blob *)blob->data;
+	blob_data = blob->data;
 	blob_data->version = FORMAT_BLOB_CURRENT;
 	blob_data->count_formats = plane->format_count;
 	blob_data->formats_offset = sizeof(struct drm_format_modifier_blob);
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm: Remove now pointelss blob->data casts

[Sharma, Shashank]

Regards

Shashank


On 2/24/2018 12:55 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that blob->data is void* again we don't need the casts anymore.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/drm_atomic.c        | 3 +--
>   drivers/gpu/drm/drm_atomic_helper.c | 2 +-
>   drivers/gpu/drm/drm_edid.c          | 3 +--
>   drivers/gpu/drm/drm_fb_helper.c     | 2 +-
>   drivers/gpu/drm/drm_plane.c         | 2 +-
>   5 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 46733d534587..8945357212ba 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -391,8 +391,7 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
>   	if (blob) {
>   		if (blob->length != sizeof(struct drm_mode_modeinfo) ||
>   		    drm_mode_convert_umode(state->crtc->dev, &state->mode,
> -		                           (const struct drm_mode_modeinfo *)
> -		                            blob->data))
> +					   blob->data))
>   			return -EINVAL;
>   
>   		state->mode_blob = drm_property_blob_get(blob);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ae3cbfe9e01c..6211a1b20405 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3816,7 +3816,7 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>   	}
>   
>   	/* Prepare GAMMA_LUT with the legacy values. */
> -	blob_data = (struct drm_color_lut *) blob->data;
> +	blob_data = blob->data;
>   	for (i = 0; i < size; i++) {
>   		blob_data[i].red = red[i];
>   		blob_data[i].green = green[i];
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 788fee4b4bf9..134069f36482 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1575,8 +1575,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>   	struct edid *override = NULL;
>   
>   	if (connector->override_edid)
> -		override = drm_edid_duplicate((const struct edid *)
> -					      connector->edid_blob_ptr->data);
> +		override = drm_edid_duplicate(connector->edid_blob_ptr->data);
>   
>   	if (!override)
>   		override = drm_load_edid_firmware(connector);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 035784ddd133..0646b108030b 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1351,7 +1351,7 @@ static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
>   	if (IS_ERR(gamma_lut))
>   		return gamma_lut;
>   
> -	lut = (struct drm_color_lut *)gamma_lut->data;
> +	lut = gamma_lut->data;
>   	if (cmap->start || cmap->len != size) {
>   		u16 *r = crtc->gamma_store;
>   		u16 *g = r + crtc->gamma_size;
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 09de6ecb3968..9851616cf0f3 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -104,7 +104,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
>   	if (IS_ERR(blob))
>   		return -1;
>   
> -	blob_data = (struct drm_format_modifier_blob *)blob->data;
> +	blob_data = blob->data;
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>   	blob_data->version = FORMAT_BLOB_CURRENT;
>   	blob_data->count_formats = plane->format_count;
>   	blob_data->formats_offset = sizeof(struct drm_format_modifier_blob);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/6] drm: Verify gamma/degamma LUT size

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

While we want to potentially support multiple different gamma/degamma
LUT sizes we can (and should) at least check that the blob length
is a multiple of the LUT entry size.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8945357212ba..933edec0299d 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -413,6 +413,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
 					 struct drm_property_blob **blob,
 					 uint64_t blob_id,
 					 ssize_t expected_size,
+					 ssize_t expected_size_mod,
 					 bool *replaced)
 {
 	struct drm_property_blob *new_blob = NULL;
@@ -422,7 +423,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
 		if (new_blob == NULL)
 			return -EINVAL;
 
-		if (expected_size > 0 && expected_size != new_blob->length) {
+		if (expected_size > 0 &&
+		    new_blob->length != expected_size) {
+			drm_property_blob_put(new_blob);
+			return -EINVAL;
+		}
+		if (expected_size_mod > 0 &&
+		    new_blob->length % expected_size_mod != 0) {
 			drm_property_blob_put(new_blob);
 			return -EINVAL;
 		}
@@ -470,7 +477,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->degamma_lut,
 					val,
-					-1,
+					-1, sizeof(struct drm_color_lut),
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
@@ -478,7 +485,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->ctm,
 					val,
-					sizeof(struct drm_color_ctm),
+					sizeof(struct drm_color_ctm), -1,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
@@ -486,7 +493,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->gamma_lut,
 					val,
-					-1,
+					-1, sizeof(struct drm_color_lut),
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/6] drm: Verify gamma/degamma LUT size

[Sharma, Shashank]

Regards

Shashank


On 2/24/2018 12:55 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> While we want to potentially support multiple different gamma/degamma
> LUT sizes we can (and should) at least check that the blob length
> is a multiple of the LUT entry size.
I dint understand the exact idea behind doing this, how is this going to 
benefit ? May be a bit more description ?
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/drm_atomic.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 8945357212ba..933edec0299d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -413,6 +413,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
>   					 struct drm_property_blob **blob,
>   					 uint64_t blob_id,
>   					 ssize_t expected_size,
> +					 ssize_t expected_size_mod,
>   					 bool *replaced)
>   {
>   	struct drm_property_blob *new_blob = NULL;
> @@ -422,7 +423,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
>   		if (new_blob == NULL)
>   			return -EINVAL;
>   
> -		if (expected_size > 0 && expected_size != new_blob->length) {
> +		if (expected_size > 0 &&
> +		    new_blob->length != expected_size) {
> +			drm_property_blob_put(new_blob);
> +			return -EINVAL;
> +		}
One line needed here, matching the previous if() pattern
> +		if (expected_size_mod > 0 &&
> +		    new_blob->length % expected_size_mod != 0) {
>   			drm_property_blob_put(new_blob);
>   			return -EINVAL;
>   		}
> @@ -470,7 +477,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   		ret = drm_atomic_replace_property_blob_from_id(dev,
>   					&state->degamma_lut,
>   					val,
> -					-1,
> +					-1, sizeof(struct drm_color_lut),
>   					&replaced);
>   		state->color_mgmt_changed |= replaced;
>   		return ret;
> @@ -478,7 +485,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   		ret = drm_atomic_replace_property_blob_from_id(dev,
>   					&state->ctm,
>   					val,
> -					sizeof(struct drm_color_ctm),
> +					sizeof(struct drm_color_ctm), -1,
>   					&replaced);
>   		state->color_mgmt_changed |= replaced;
>   		return ret;
> @@ -486,7 +493,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   		ret = drm_atomic_replace_property_blob_from_id(dev,
>   					&state->gamma_lut,
>   					val,
> -					-1,
> +					-1, sizeof(struct drm_color_lut),
>   					&replaced);
>   		state->color_mgmt_changed |= replaced;
>   		return ret;

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm: Verify gamma/degamma LUT size

[Ville Syrjälä]

On Thu, Mar 01, 2018 at 06:43:21PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 2/24/2018 12:55 AM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > While we want to potentially support multiple different gamma/degamma
> > LUT sizes we can (and should) at least check that the blob length
> > is a multiple of the LUT entry size.
> I dint understand the exact idea behind doing this, how is this going to 
> benefit ? May be a bit more description ?

The benefit is rejecting garbage fed in from userspace.

> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/drm_atomic.c | 15 +++++++++++----
> >   1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 8945357212ba..933edec0299d 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -413,6 +413,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> >   					 struct drm_property_blob **blob,
> >   					 uint64_t blob_id,
> >   					 ssize_t expected_size,
> > +					 ssize_t expected_size_mod,
> >   					 bool *replaced)
> >   {
> >   	struct drm_property_blob *new_blob = NULL;
> > @@ -422,7 +423,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> >   		if (new_blob == NULL)
> >   			return -EINVAL;
> >   
> > -		if (expected_size > 0 && expected_size != new_blob->length) {
> > +		if (expected_size > 0 &&
> > +		    new_blob->length != expected_size) {
> > +			drm_property_blob_put(new_blob);
> > +			return -EINVAL;
> > +		}
> One line needed here, matching the previous if() pattern

What line? Don't understand.

> > +		if (expected_size_mod > 0 &&
> > +		    new_blob->length % expected_size_mod != 0) {
> >   			drm_property_blob_put(new_blob);
> >   			return -EINVAL;
> >   		}
> > @@ -470,7 +477,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >   		ret = drm_atomic_replace_property_blob_from_id(dev,
> >   					&state->degamma_lut,
> >   					val,
> > -					-1,
> > +					-1, sizeof(struct drm_color_lut),
> >   					&replaced);
> >   		state->color_mgmt_changed |= replaced;
> >   		return ret;
> > @@ -478,7 +485,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >   		ret = drm_atomic_replace_property_blob_from_id(dev,
> >   					&state->ctm,
> >   					val,
> > -					sizeof(struct drm_color_ctm),
> > +					sizeof(struct drm_color_ctm), -1,
> >   					&replaced);
> >   		state->color_mgmt_changed |= replaced;
> >   		return ret;
> > @@ -486,7 +493,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >   		ret = drm_atomic_replace_property_blob_from_id(dev,
> >   					&state->gamma_lut,
> >   					val,
> > -					-1,
> > +					-1, sizeof(struct drm_color_lut),
> >   					&replaced);
> >   		state->color_mgmt_changed |= replaced;
> >   		return ret;

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/6] drm: Verify gamma/degamma LUT size

[Sharma, Shashank]

Regards

Shashank


On 3/1/2018 6:54 PM, Ville Syrjälä wrote:
> On Thu, Mar 01, 2018 at 06:43:21PM +0530, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 2/24/2018 12:55 AM, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> While we want to potentially support multiple different gamma/degamma
>>> LUT sizes we can (and should) at least check that the blob length
>>> is a multiple of the LUT entry size.
>> I dint understand the exact idea behind doing this, how is this going to
>> benefit ? May be a bit more description ?
> The benefit is rejecting garbage fed in from userspace.
>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/drm_atomic.c | 15 +++++++++++----
>>>    1 file changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>> index 8945357212ba..933edec0299d 100644
>>> --- a/drivers/gpu/drm/drm_atomic.c
>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>> @@ -413,6 +413,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
>>>    					 struct drm_property_blob **blob,
>>>    					 uint64_t blob_id,
>>>    					 ssize_t expected_size,
>>> +					 ssize_t expected_size_mod,
>>>    					 bool *replaced)
>>>    {
>>>    	struct drm_property_blob *new_blob = NULL;
>>> @@ -422,7 +423,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
>>>    		if (new_blob == NULL)
>>>    			return -EINVAL;
>>>    
>>> -		if (expected_size > 0 && expected_size != new_blob->length) {
>>> +		if (expected_size > 0 &&
>>> +		    new_blob->length != expected_size) {
>>> +			drm_property_blob_put(new_blob);
>>> +			return -EINVAL;
>>> +		}
>> One line needed here, matching the previous if() pattern
> What line? Don't understand.
I mean, I can see a blank line before previous if() condition, so lets 
keep the same pattern for this if() too

- Shashank
>>> +		if (expected_size_mod > 0 &&
>>> +		    new_blob->length % expected_size_mod != 0) {
>>>    			drm_property_blob_put(new_blob);
>>>    			return -EINVAL;
>>>    		}
>>> @@ -470,7 +477,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>>    		ret = drm_atomic_replace_property_blob_from_id(dev,
>>>    					&state->degamma_lut,
>>>    					val,
>>> -					-1,
>>> +					-1, sizeof(struct drm_color_lut),
>>>    					&replaced);
>>>    		state->color_mgmt_changed |= replaced;
>>>    		return ret;
>>> @@ -478,7 +485,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>>    		ret = drm_atomic_replace_property_blob_from_id(dev,
>>>    					&state->ctm,
>>>    					val,
>>> -					sizeof(struct drm_color_ctm),
>>> +					sizeof(struct drm_color_ctm), -1,
>>>    					&replaced);
>>>    		state->color_mgmt_changed |= replaced;
>>>    		return ret;
>>> @@ -486,7 +493,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>>    		ret = drm_atomic_replace_property_blob_from_id(dev,
>>>    					&state->gamma_lut,
>>>    					val,
>>> -					-1,
>>> +					-1, sizeof(struct drm_color_lut),
>>>    					&replaced);
>>>    		state->color_mgmt_changed |= replaced;
>>>    		return ret;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/6] drm: Verify gamma/degamma LUT size

[Ville Syrjälä]

On Thu, Mar 01, 2018 at 07:58:07PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 3/1/2018 6:54 PM, Ville Syrjälä wrote:
> > On Thu, Mar 01, 2018 at 06:43:21PM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 2/24/2018 12:55 AM, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> While we want to potentially support multiple different gamma/degamma
> >>> LUT sizes we can (and should) at least check that the blob length
> >>> is a multiple of the LUT entry size.
> >> I dint understand the exact idea behind doing this, how is this going to
> >> benefit ? May be a bit more description ?
> > The benefit is rejecting garbage fed in from userspace.
> >
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>    drivers/gpu/drm/drm_atomic.c | 15 +++++++++++----
> >>>    1 file changed, 11 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >>> index 8945357212ba..933edec0299d 100644
> >>> --- a/drivers/gpu/drm/drm_atomic.c
> >>> +++ b/drivers/gpu/drm/drm_atomic.c
> >>> @@ -413,6 +413,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> >>>    					 struct drm_property_blob **blob,
> >>>    					 uint64_t blob_id,
> >>>    					 ssize_t expected_size,
> >>> +					 ssize_t expected_size_mod,
> >>>    					 bool *replaced)
> >>>    {
> >>>    	struct drm_property_blob *new_blob = NULL;
> >>> @@ -422,7 +423,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> >>>    		if (new_blob == NULL)
> >>>    			return -EINVAL;
> >>>    
> >>> -		if (expected_size > 0 && expected_size != new_blob->length) {
> >>> +		if (expected_size > 0 &&
> >>> +		    new_blob->length != expected_size) {
> >>> +			drm_property_blob_put(new_blob);
> >>> +			return -EINVAL;
> >>> +		}
> >> One line needed here, matching the previous if() pattern
> > What line? Don't understand.
> I mean, I can see a blank line before previous if() condition, so lets 
> keep the same pattern for this if() too

OTOH the two ifs are related so maybe just keep them together? Doesn't
actually matter to me though.

> 
> - Shashank
> >>> +		if (expected_size_mod > 0 &&
> >>> +		    new_blob->length % expected_size_mod != 0) {
> >>>    			drm_property_blob_put(new_blob);
> >>>    			return -EINVAL;
> >>>    		}
> >>> @@ -470,7 +477,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>>    		ret = drm_atomic_replace_property_blob_from_id(dev,
> >>>    					&state->degamma_lut,
> >>>    					val,
> >>> -					-1,
> >>> +					-1, sizeof(struct drm_color_lut),
> >>>    					&replaced);
> >>>    		state->color_mgmt_changed |= replaced;
> >>>    		return ret;
> >>> @@ -478,7 +485,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>>    		ret = drm_atomic_replace_property_blob_from_id(dev,
> >>>    					&state->ctm,
> >>>    					val,
> >>> -					sizeof(struct drm_color_ctm),
> >>> +					sizeof(struct drm_color_ctm), -1,
> >>>    					&replaced);
> >>>    		state->color_mgmt_changed |= replaced;
> >>>    		return ret;
> >>> @@ -486,7 +493,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>>    		ret = drm_atomic_replace_property_blob_from_id(dev,
> >>>    					&state->gamma_lut,
> >>>    					val,
> >>> -					-1,
> >>> +					-1, sizeof(struct drm_color_lut),
> >>>    					&replaced);
> >>>    		state->color_mgmt_changed |= replaced;
> >>>    		return ret;

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 3/6] drm: Verify gamma/degamma LUT size

[Daniel Vetter]

On Fri, Feb 23, 2018 at 09:25:03PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> While we want to potentially support multiple different gamma/degamma
> LUT sizes we can (and should) at least check that the blob length
> is a multiple of the LUT entry size.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 8945357212ba..933edec0299d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -413,6 +413,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
>  					 struct drm_property_blob **blob,
>  					 uint64_t blob_id,
>  					 ssize_t expected_size,
> +					 ssize_t expected_size_mod,

Needs kerneldoc, and I'm not sure it's the most descriptive name. Maybe
expected_array_element_size?

With or without the bikeshed, but with the kerneldoc fixed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Up to this patch in the series.
-Daniel

>  					 bool *replaced)
>  {
>  	struct drm_property_blob *new_blob = NULL;
> @@ -422,7 +423,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
>  		if (new_blob == NULL)
>  			return -EINVAL;
>  
> -		if (expected_size > 0 && expected_size != new_blob->length) {
> +		if (expected_size > 0 &&
> +		    new_blob->length != expected_size) {
> +			drm_property_blob_put(new_blob);
> +			return -EINVAL;
> +		}
> +		if (expected_size_mod > 0 &&
> +		    new_blob->length % expected_size_mod != 0) {
>  			drm_property_blob_put(new_blob);
>  			return -EINVAL;
>  		}
> @@ -470,7 +477,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->degamma_lut,
>  					val,
> -					-1,
> +					-1, sizeof(struct drm_color_lut),
>  					&replaced);
>  		state->color_mgmt_changed |= replaced;
>  		return ret;
> @@ -478,7 +485,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->ctm,
>  					val,
> -					sizeof(struct drm_color_ctm),
> +					sizeof(struct drm_color_ctm), -1,
>  					&replaced);
>  		state->color_mgmt_changed |= replaced;
>  		return ret;
> @@ -486,7 +493,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->gamma_lut,
>  					val,
> -					-1,
> +					-1, sizeof(struct drm_color_lut),
>  					&replaced);
>  		state->color_mgmt_changed |= replaced;
>  		return ret;
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/6] drm: Verify gamma/degamma LUT size

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

While we want to potentially support multiple different gamma/degamma
LUT sizes we can (and should) at least check that the blob length
is a multiple of the LUT entry size.

v2: s/expected_size_mod/expected_elem_size/ (Daniel)
    Add kernel doc (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/gpu/drm-kms.rst |  3 +++
 drivers/gpu/drm/drm_atomic.c  | 39 +++++++++++++++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 56a3780e39b8..1dffd1ac4cd4 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -286,6 +286,9 @@ Atomic Mode Setting Function Reference
 .. kernel-doc:: drivers/gpu/drm/drm_atomic.c
    :export:
 
+.. kernel-doc:: drivers/gpu/drm/drm_atomic.c
+   :internal:
+
 CRTC Abstraction
 ================
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 01060669dd49..a55f611d2a6f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -408,11 +408,36 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 }
 EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
 
+/**
+ * drm_atomic_replace_property_blob_from_id - lookup the new blob and replace the old one with it
+ * @dev: DRM device
+ * @blob: a pointer to the member blob to be replaced
+ * @blob_id: ID of the new blob
+ * @expected_size: total expected size of the blob data (in bytes)
+ * @expected_elem_size: expected element size of the blob data (in bytes)
+ * @replaced: did the blob get replaced?
+ *
+ * Replace @blob with another blob with the ID @blob_id. If @blob_id is zero
+ * @blob becomes NULL.
+ *
+ * If @expected_size is positive the new blob length is expected to be equal
+ * to @expected_size bytes. If @expected_elem_size is positive the new blob
+ * length is expected to be a multiple of @expected_elem_size bytes. Otherwise
+ * an error is returned.
+ *
+ * @replaced will indicate to the caller whether the blob was replaced or not.
+ * If the old and new blobs we in fact the same blob @replaced will be false
+ * otherwise it will be true.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure.
+ */
 static int
 drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
 					 struct drm_property_blob **blob,
 					 uint64_t blob_id,
 					 ssize_t expected_size,
+					 ssize_t expected_elem_size,
 					 bool *replaced)
 {
 	struct drm_property_blob *new_blob = NULL;
@@ -422,7 +447,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
 		if (new_blob == NULL)
 			return -EINVAL;
 
-		if (expected_size > 0 && expected_size != new_blob->length) {
+		if (expected_size > 0 &&
+		    new_blob->length != expected_size) {
+			drm_property_blob_put(new_blob);
+			return -EINVAL;
+		}
+		if (expected_elem_size > 0 &&
+		    new_blob->length % expected_elem_size != 0) {
 			drm_property_blob_put(new_blob);
 			return -EINVAL;
 		}
@@ -470,7 +501,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->degamma_lut,
 					val,
-					-1,
+					-1, sizeof(struct drm_color_lut),
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
@@ -478,7 +509,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->ctm,
 					val,
-					sizeof(struct drm_color_ctm),
+					sizeof(struct drm_color_ctm), -1,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
@@ -486,7 +517,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->gamma_lut,
 					val,
-					-1,
+					-1, sizeof(struct drm_color_lut),
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
-- 
2.16.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/6] drm: Introduce drm_color_lut_size()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Provide a small helper to convert the blob length in bytes
to the number of LUT entries.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/drm/drm_color_mgmt.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 03a59cbce621..7ddf4457f3c1 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -37,4 +37,9 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
 				 int gamma_size);
 
+static inline int drm_color_lut_size(const struct drm_property_blob *blob)
+{
+	return blob->length / sizeof(struct drm_color_lut);
+}
+
 #endif
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm: Introduce drm_color_lut_size()

[Daniel Vetter]

On Fri, Feb 23, 2018 at 09:25:04PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Provide a small helper to convert the blob length in bytes
> to the number of LUT entries.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  include/drm/drm_color_mgmt.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 03a59cbce621..7ddf4457f3c1 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -37,4 +37,9 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
>  				 int gamma_size);
>  

Add a bit of kerneldoc and you get my

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

for this and all remaining patches.
-Daniel

> +static inline int drm_color_lut_size(const struct drm_property_blob *blob)
> +{
> +	return blob->length / sizeof(struct drm_color_lut);
> +}
> +
>  #endif
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/6] drm: Introduce drm_color_lut_size()

[Ville Syrjälä]

On Tue, Mar 06, 2018 at 08:54:21AM +0100, Daniel Vetter wrote:
> On Fri, Feb 23, 2018 at 09:25:04PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Provide a small helper to convert the blob length in bytes
> > to the number of LUT entries.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  include/drm/drm_color_mgmt.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index 03a59cbce621..7ddf4457f3c1 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -37,4 +37,9 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >  int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
> >  				 int gamma_size);
> >  
> 
> Add a bit of kerneldoc and you get my
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> for this and all remaining patches.

Entire series pushed to drm-misc-next. Thanks for the reviews.

> -Daniel
> 
> > +static inline int drm_color_lut_size(const struct drm_property_blob *blob)
> > +{
> > +	return blob->length / sizeof(struct drm_color_lut);
> > +}
> > +
> >  #endif
> > -- 
> > 2.13.6
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/6] drm: Introduce drm_color_lut_size()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Provide a small helper to convert the blob length in bytes
to the number of LUT entries.

v2: Add kerneldoc (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/drm/drm_color_mgmt.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index b3b6d302ca8c..44f04233e3db 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -38,6 +38,18 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
 				 int gamma_size);
 
+/**
+ * drm_color_lut_size - calculate the number of entries in the LUT
+ * @blob: blob containing the LUT
+ *
+ * Returns:
+ * The number of entries in the color LUT stored in @blob.
+ */
+static inline int drm_color_lut_size(const struct drm_property_blob *blob)
+{
+	return blob->length / sizeof(struct drm_color_lut);
+}
+
 enum drm_color_encoding {
 	DRM_COLOR_YCBCR_BT601,
 	DRM_COLOR_YCBCR_BT709,
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/6] drm/i915: Remove the blob->data casts

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that blob->data is void* again we don't need to cast it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index a383d993b844..e8ede69754a9 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -144,8 +144,7 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
 		i9xx_load_ycbcr_conversion_matrix(intel_crtc);
 		return;
 	} else if (crtc_state->ctm) {
-		struct drm_color_ctm *ctm =
-			(struct drm_color_ctm *)crtc_state->ctm->data;
+		struct drm_color_ctm *ctm = crtc_state->ctm->data;
 		uint64_t input[9] = { 0, };
 
 		if (intel_crtc_state->limited_color_range) {
@@ -254,8 +253,7 @@ static void cherryview_load_csc_matrix(struct drm_crtc_state *state)
 	uint32_t mode;
 
 	if (state->ctm) {
-		struct drm_color_ctm *ctm =
-			(struct drm_color_ctm *) state->ctm->data;
+		struct drm_color_ctm *ctm = state->ctm->data;
 		uint16_t coeffs[9] = { 0, };
 		int i;
 
@@ -322,7 +320,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
 	}
 
 	if (blob) {
-		struct drm_color_lut *lut = (struct drm_color_lut *) blob->data;
+		struct drm_color_lut *lut = blob->data;
 		for (i = 0; i < 256; i++) {
 			uint32_t word =
 				(drm_color_lut_extract(lut[i].red, 8) << 16) |
@@ -392,8 +390,7 @@ static void bdw_load_degamma_lut(struct drm_crtc_state *state)
 		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
 
 	if (state->degamma_lut) {
-		struct drm_color_lut *lut =
-			(struct drm_color_lut *) state->degamma_lut->data;
+		struct drm_color_lut *lut = state->degamma_lut->data;
 
 		for (i = 0; i < lut_size; i++) {
 			uint32_t word =
@@ -427,8 +424,7 @@ static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
 		   offset);
 
 	if (state->gamma_lut) {
-		struct drm_color_lut *lut =
-			(struct drm_color_lut *) state->gamma_lut->data;
+		struct drm_color_lut *lut = state->gamma_lut->data;
 
 		for (i = 0; i < lut_size; i++) {
 			uint32_t word =
@@ -560,7 +556,7 @@ static void cherryview_load_luts(struct drm_crtc_state *state)
 	}
 
 	if (state->degamma_lut) {
-		lut = (struct drm_color_lut *) state->degamma_lut->data;
+		lut = state->degamma_lut->data;
 		lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
 		for (i = 0; i < lut_size; i++) {
 			/* Write LUT in U0.14 format. */
@@ -575,7 +571,7 @@ static void cherryview_load_luts(struct drm_crtc_state *state)
 	}
 
 	if (state->gamma_lut) {
-		lut = (struct drm_color_lut *) state->gamma_lut->data;
+		lut = state->gamma_lut->data;
 		lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 		for (i = 0; i < lut_size; i++) {
 			/* Write LUT in U0.10 format. */
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 5/6] drm/i915: Remove the blob->data casts

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that blob->data is void* again we don't need to cast it.

v2: Rebase

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_color.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 89ab0f70aa22..92f148832a89 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -153,8 +153,7 @@ static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state)
 		ilk_load_ycbcr_conversion_matrix(intel_crtc);
 		return;
 	} else if (crtc_state->ctm) {
-		struct drm_color_ctm *ctm =
-			(struct drm_color_ctm *)crtc_state->ctm->data;
+		struct drm_color_ctm *ctm = crtc_state->ctm->data;
 		const u64 *input;
 		u64 temp[9];
 
@@ -262,8 +261,7 @@ static void cherryview_load_csc_matrix(struct drm_crtc_state *state)
 	uint32_t mode;
 
 	if (state->ctm) {
-		struct drm_color_ctm *ctm =
-			(struct drm_color_ctm *) state->ctm->data;
+		struct drm_color_ctm *ctm = state->ctm->data;
 		uint16_t coeffs[9] = { 0, };
 		int i;
 
@@ -330,7 +328,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
 	}
 
 	if (blob) {
-		struct drm_color_lut *lut = (struct drm_color_lut *) blob->data;
+		struct drm_color_lut *lut = blob->data;
 		for (i = 0; i < 256; i++) {
 			uint32_t word =
 				(drm_color_lut_extract(lut[i].red, 8) << 16) |
@@ -400,8 +398,7 @@ static void bdw_load_degamma_lut(struct drm_crtc_state *state)
 		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
 
 	if (state->degamma_lut) {
-		struct drm_color_lut *lut =
-			(struct drm_color_lut *) state->degamma_lut->data;
+		struct drm_color_lut *lut = state->degamma_lut->data;
 
 		for (i = 0; i < lut_size; i++) {
 			uint32_t word =
@@ -435,8 +432,7 @@ static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
 		   offset);
 
 	if (state->gamma_lut) {
-		struct drm_color_lut *lut =
-			(struct drm_color_lut *) state->gamma_lut->data;
+		struct drm_color_lut *lut = state->gamma_lut->data;
 
 		for (i = 0; i < lut_size; i++) {
 			uint32_t word =
@@ -568,7 +564,7 @@ static void cherryview_load_luts(struct drm_crtc_state *state)
 	}
 
 	if (state->degamma_lut) {
-		lut = (struct drm_color_lut *) state->degamma_lut->data;
+		lut = state->degamma_lut->data;
 		lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
 		for (i = 0; i < lut_size; i++) {
 			/* Write LUT in U0.14 format. */
@@ -583,7 +579,7 @@ static void cherryview_load_luts(struct drm_crtc_state *state)
 	}
 
 	if (state->gamma_lut) {
-		lut = (struct drm_color_lut *) state->gamma_lut->data;
+		lut = state->gamma_lut->data;
 		lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 		for (i = 0; i < lut_size; i++) {
 			/* Write LUT in U0.10 format. */
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/6] drm/i915: Use drm_color_lut_size()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Avoid all the sizeof(drm_color_lut) business by using
drm_color_lut_size() to convert the blob length into
number of LUT entries.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index e8ede69754a9..029c2c931fab 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -39,7 +39,7 @@
 #define CTM_COEFF_NEGATIVE(coeff)	(((coeff) & CTM_COEFF_SIGN) != 0)
 #define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
 
-#define LEGACY_LUT_LENGTH		(sizeof(struct drm_color_lut) * 256)
+#define LEGACY_LUT_LENGTH		256
 
 /* Post offset values for RGB->YCBCR conversion */
 #define POSTOFF_RGB_TO_YUV_HI 0x800
@@ -79,7 +79,7 @@ static bool crtc_state_is_legacy_gamma(struct drm_crtc_state *state)
 	return !state->degamma_lut &&
 		!state->ctm &&
 		state->gamma_lut &&
-		state->gamma_lut->length == LEGACY_LUT_LENGTH;
+		drm_color_lut_size(state->gamma_lut) == LEGACY_LUT_LENGTH;
 }
 
 /*
@@ -611,19 +611,17 @@ int intel_color_check(struct drm_crtc *crtc,
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	size_t gamma_length, degamma_length;
 
-	degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size *
-		sizeof(struct drm_color_lut);
-	gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size *
-		sizeof(struct drm_color_lut);
+	degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+	gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 
 	/*
 	 * We allow both degamma & gamma luts at the right size or
 	 * NULL.
 	 */
 	if ((!crtc_state->degamma_lut ||
-	     crtc_state->degamma_lut->length == degamma_length) &&
+	     drm_color_lut_size(crtc_state->degamma_lut) == degamma_length) &&
 	    (!crtc_state->gamma_lut ||
-	     crtc_state->gamma_lut->length == gamma_length))
+	     drm_color_lut_size(crtc_state->gamma_lut) == gamma_length))
 		return 0;
 
 	/*
-- 
2.13.6

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/6] Revert "drm: Use a flexible array member for blob property data"

[Sharma, Shashank]

Regards

Shashank


On 2/24/2018 12:55 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Using a flexible array for the blob data was a mistake by me. It
> forces all users of the blob data to cast blob->data to something
> else. void* is clearly superior so let's go back to the original
> scheme.
>
> Not a clean revert as the code has moved.
>
> This reverts commit d63f5e6bf6f2a1573ea39c9937cdf5ab0b3a4b77.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/drm_property.c | 1 +
>   include/drm/drm_property.h     | 2 +-
>   2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index bae50e6b819d..0f6620fea3de 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -550,6 +550,7 @@ drm_property_create_blob(struct drm_device *dev, size_t length,
>   	/* This must be explicitly initialised, so we can safely call list_del
>   	 * on it in the removal handler, even if it isn't in a file list. */
>   	INIT_LIST_HEAD(&blob->head_file);
> +	blob->data = (void *)blob + sizeof(*blob);
>   	blob->length = length;
>   	blob->dev = dev;
>   
> diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
> index 8a522b4bed40..265fd1f2e112 100644
> --- a/include/drm/drm_property.h
> +++ b/include/drm/drm_property.h
> @@ -209,7 +209,7 @@ struct drm_property_blob {
>   	struct list_head head_global;
>   	struct list_head head_file;
>   	size_t length;
> -	unsigned char data[];
> +	void *data;
>   };
>   
>   struct drm_prop_enum_list {
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

Regards
Shashank
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel