Hi
Am 10.12.25 um 20:42 schrieb Dmitry Baryshkov:
The function drm_property_replace_blob_from_id() allows checking whether
the blob size is equal to a predefined value. In case of variable-size
properties (like the gamma / degamma LUTs) we might want to check for
the blob size against the maximum, allowing properties of the size
lesser than the max supported by the hardware. Extend the function in
order to support such checks.
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 5 +++++
drivers/gpu/drm/drm_atomic_uapi.c | 7 +++++--
drivers/gpu/drm/drm_property.c | 11 +++++++++++
include/drm/drm_property.h | 1 +
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index e027798ece03..d19631b5d9e1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1677,6 +1677,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
&dm_plane_state->degamma_lut,
val, -1,
sizeof(struct
drm_color_lut),
+ 0,
The rest of the API uses -1 for an invalid/unknown argument. I'd stick
with that instead of using 0.
&replaced);
dm_plane_state->base.color_mgmt_changed |= replaced;
return ret;
@@ -1695,6 +1696,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
&dm_plane_state->ctm,
val,
sizeof(struct
drm_color_ctm_3x4), -1,
+ 0,
&replaced);
dm_plane_state->base.color_mgmt_changed |= replaced;
return ret;
@@ -1703,6 +1705,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
&dm_plane_state->shaper_lut,
val, -1,
sizeof(struct
drm_color_lut),
+ 0,
&replaced);
dm_plane_state->base.color_mgmt_changed |= replaced;
return ret;
@@ -1716,6 +1719,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
&dm_plane_state->lut3d,
val, -1,
sizeof(struct
drm_color_lut),
+ 0,
&replaced);
dm_plane_state->base.color_mgmt_changed |= replaced;
return ret;
@@ -1724,6 +1728,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
&dm_plane_state->blend_lut,
val, -1,
sizeof(struct
drm_color_lut),
+ 0,
&replaced);
dm_plane_state->base.color_mgmt_changed |= replaced;
return ret;
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 85dbdaa4a2e2..7d2076f1006e 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -388,6 +388,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
*crtc,
&state->degamma_lut,
val,
-1, sizeof(struct drm_color_lut),
+ 0,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
@@ -395,7 +396,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
*crtc,
ret = drm_property_replace_blob_from_id(dev,
&state->ctm,
val,
- sizeof(struct drm_color_ctm), -1,
+ sizeof(struct drm_color_ctm), -1, 0,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
@@ -404,6 +405,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
*crtc,
&state->gamma_lut,
val,
-1, sizeof(struct drm_color_lut),
+ 0,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
@@ -546,6 +548,7 @@ static int drm_atomic_plane_set_property(struct drm_plane
*plane,
val,
-1,
sizeof(struct drm_mode_rect),
+ 0,
&replaced);
return ret;
} else if (property == plane->scaling_filter_property) {
@@ -741,7 +744,7 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
ret = drm_property_replace_blob_from_id(dev,
&state->hdr_output_metadata,
val,
- sizeof(struct hdr_output_metadata), -1,
+ sizeof(struct hdr_output_metadata), -1, 0,
&replaced);
return ret;
} else if (property == config->aspect_ratio_property) {
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 596272149a35..5befe443135d 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -759,6 +759,7 @@ EXPORT_SYMBOL(drm_property_replace_blob);
* @blob_id: the id of the new blob to replace with
* @expected_size: expected size of the blob property
* @expected_elem_size: expected size of an element in the blob property
+ * @max_size: the maximum size of the blob property for variable-size blobs
* @replaced: if the blob was in fact replaced
*
* Look up the new blob from id, take its reference, check expected sizes of
@@ -773,6 +774,7 @@ int drm_property_replace_blob_from_id(struct drm_device
*dev,
uint64_t blob_id,
ssize_t expected_size,
ssize_t expected_elem_size,
+ ssize_t max_size,
I'd put max_size before expected_size, so that the size of individual
elements isn't located in the middle of them.
bool *replaced)
{
struct drm_property_blob *new_blob = NULL;
@@ -801,6 +803,15 @@ int drm_property_replace_blob_from_id(struct drm_device
*dev,
drm_property_blob_put(new_blob);
return -EINVAL;
}
+
+ if (max_size > 0 &&
+ new_blob->length > max_size) {
+ drm_dbg_atomic(dev,
+ "[BLOB:%d] length %zu greater than max
%zu\n",
+ new_blob->base.id, new_blob->length,
max_size);
+ drm_property_blob_put(new_blob);
+ return -EINVAL;
+ }
I'd first test for max_size before testing for expected size.
And shouldn't you also test for (max_size % expected_elem_size == 0)?
Best regards
Thomas
}
*replaced |= drm_property_replace_blob(blob, new_blob);
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index 082f29156b3e..aa49b5a42bb5 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -284,6 +284,7 @@ int drm_property_replace_blob_from_id(struct drm_device
*dev,
uint64_t blob_id,
ssize_t expected_size,
ssize_t expected_elem_size,
+ ssize_t max_size,
bool *replaced);
int drm_property_replace_global_blob(struct drm_device *dev,
struct drm_property_blob **replace,
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
