* [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
[not found] <20210929194012.3433306-1-markyacoub@chromium.org>
@ 2021-10-13 18:12 ` Mark Yacoub
2021-10-13 18:12 ` [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check Mark Yacoub
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Mark Yacoub @ 2021-10-13 18:12 UTC (permalink / raw)
To: dri-devel, amd-gfx, intel-gfx
Cc: seanpaul, harry.wentland, Mark Yacoub, Mark Yacoub,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Matthias Brugger, linux-kernel, linux-arm-kernel,
linux-mediatek
From: Mark Yacoub <markyacoub@google.com>
[Why]
1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
or Degamma props in the new CRTC state, allowing any invalid size to
be passed on.
2. Each driver has its own LUT size, which could also be different for
legacy users.
[How]
1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
assigned by the driver when it's initializing its color and CTM
management.
2. Create drm_atomic_helper_check_crtc which is called by
drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
they match the sizes in the new CRTC state.
3. Rename older lut checks that test for the color channels to indicate
it's a channel check. It's not included in drm_atomic_helper_check_crtc
as it's hardware specific and is to be called by the driver.
4. As the LUT size check now happens in drm_atomic_helper_check, remove
the lut check in intel_color.c
Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL)
v1:
1. Fix typos
2. Remove the LUT size check from intel driver
3. Rename old LUT check to indicate it's a channel change
Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
---
drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++
drivers/gpu/drm/drm_color_mgmt.c | 14 ++---
drivers/gpu/drm/i915/display/intel_color.c | 14 ++---
include/drm/drm_atomic_helper.h | 1 +
include/drm/drm_color_mgmt.h | 7 +--
include/drm/drm_crtc.h | 11 ++++
6 files changed, 89 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index bc3487964fb5e..5feb2ad0209c3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_atomic_helper_check_planes);
+/**
+ * drm_atomic_helper_check_crtcs - validate state object for CRTC changes
+ * @state: the driver state object
+ *
+ * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
+ * state holds them.
+ *
+ * RETURNS:
+ * Zero for success or -errno
+ */
+int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *new_crtc_state;
+ int i;
+
+ for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
+ if (new_crtc_state->color_mgmt_changed &&
+ new_crtc_state->gamma_lut) {
+ uint64_t supported_lut_size = crtc->gamma_lut_size;
+ uint32_t supported_legacy_lut_size = crtc->gamma_size;
+ uint32_t new_state_lut_size =
+ drm_color_lut_size(new_crtc_state->gamma_lut);
+
+ if (new_state_lut_size != supported_lut_size &&
+ new_state_lut_size != supported_legacy_lut_size) {
+ drm_dbg_state(
+ state->dev,
+ "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n",
+ supported_lut_size,
+ supported_legacy_lut_size,
+ new_state_lut_size);
+ return -EINVAL;
+ }
+ }
+
+ if (new_crtc_state->color_mgmt_changed &&
+ new_crtc_state->degamma_lut) {
+ uint32_t new_state_lut_size =
+ drm_color_lut_size(new_crtc_state->degamma_lut);
+ uint64_t supported_lut_size = crtc->degamma_lut_size;
+
+ if (new_state_lut_size != supported_lut_size) {
+ drm_dbg_state(
+ state->dev,
+ "Invalid Degamma LUT size. Should be %u but got %u.\n",
+ supported_lut_size, new_state_lut_size);
+ return -EINVAL;
+ }
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check_crtcs);
+
/**
* drm_atomic_helper_check - validate state object
* @dev: DRM device
@@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
if (ret)
return ret;
+ ret = drm_atomic_helper_check_crtcs(state);
+ if (ret)
+ return ret;
+
if (state->legacy_cursor_update)
state->async_update = !drm_atomic_helper_async_check(dev, state);
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index bb14f488c8f6c..e5b820ce823bf 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
struct drm_mode_config *config = &dev->mode_config;
if (degamma_lut_size) {
+ crtc->degamma_lut_size = degamma_lut_size;
drm_object_attach_property(&crtc->base,
config->degamma_lut_property, 0);
drm_object_attach_property(&crtc->base,
@@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
config->ctm_property, 0);
if (gamma_lut_size) {
+ crtc->gamma_lut_size = gamma_lut_size;
drm_object_attach_property(&crtc->base,
config->gamma_lut_property, 0);
drm_object_attach_property(&crtc->base,
@@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
EXPORT_SYMBOL(drm_plane_create_color_properties);
/**
- * drm_color_lut_check - check validity of lookup table
+ * drm_color_lut_channels_check - check validity of the channels in the lookup table
* @lut: property blob containing LUT to check
* @tests: bitmask of tests to run
*
- * Helper to check whether a userspace-provided lookup table is valid and
- * satisfies hardware requirements. Drivers pass a bitmask indicating which of
- * the tests in &drm_color_lut_tests should be performed.
+ * Helper to check whether each color channel of userspace-provided lookup table is valid and
+ * satisfies hardware requirements. Drivers pass a bitmask indicating which of in
+ * &drm_color_lut_channels_tests should be performed.
*
* Returns 0 on success, -EINVAL on failure.
*/
-int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
+int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests)
{
const struct drm_color_lut *entry;
int i;
@@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
return 0;
}
-EXPORT_SYMBOL(drm_color_lut_check);
+EXPORT_SYMBOL(drm_color_lut_channels_check);
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index dab892d2251ba..a308fe52746ac 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
int gamma_length, degamma_length;
- u32 gamma_tests, degamma_tests;
+ u32 gamma_channels_tests, degamma_channels_tests;
/* Always allow legacy gamma LUT with no further checking. */
if (crtc_state_is_legacy_gamma(crtc_state))
@@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
- degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
- gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
+ degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
+ gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
- if (check_lut_size(degamma_lut, degamma_length) ||
- check_lut_size(gamma_lut, gamma_length))
- return -EINVAL;
-
- if (drm_color_lut_check(degamma_lut, degamma_tests) ||
- drm_color_lut_check(gamma_lut, gamma_tests))
+ if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) ||
+ drm_color_lut_channels_check(gamma_lut, gamma_channels_tests))
return -EINVAL;
return 0;
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 4045e2507e11c..a22d32a7a8719 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -38,6 +38,7 @@ struct drm_atomic_state;
struct drm_private_obj;
struct drm_private_state;
+int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state);
int drm_atomic_helper_check_modeset(struct drm_device *dev,
struct drm_atomic_state *state);
int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 81c298488b0c8..cb1bf361ad3e3 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
enum drm_color_range default_range);
/**
- * enum drm_color_lut_tests - hw-specific LUT tests to perform
+ * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform
*
* The drm_color_lut_check() function takes a bitmask of the values here to
* determine which tests to apply to a userspace-provided LUT.
*/
-enum drm_color_lut_tests {
+enum drm_color_lut_channels_tests {
/**
* @DRM_COLOR_LUT_EQUAL_CHANNELS:
*
@@ -119,5 +119,6 @@ enum drm_color_lut_tests {
DRM_COLOR_LUT_NON_DECREASING = BIT(1),
};
-int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests);
+int drm_color_lut_channels_check(const struct drm_property_blob *lut,
+ u32 tests);
#endif
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 2deb15d7e1610..cabd3ef1a6e32 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1072,6 +1072,17 @@ struct drm_crtc {
/** @funcs: CRTC control functions */
const struct drm_crtc_funcs *funcs;
+ /**
+ * @degamma_lut_size: Size of degamma LUT.
+ */
+ uint32_t degamma_lut_size;
+
+ /**
+ * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as
+ * X, which doesn't support large lut sizes.
+ */
+ uint32_t gamma_lut_size;
+
/**
* @gamma_size: Size of legacy gamma ramp reported to userspace. Set up
* by calling drm_mode_crtc_set_gamma_size().
--
2.33.0.882.g93a45727a2-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check
2021-10-13 18:12 ` [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate Mark Yacoub
@ 2021-10-13 18:12 ` Mark Yacoub
2021-10-14 23:42 ` Sean Paul
2021-10-26 1:26 ` [Intel-gfx] [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate Sean Paul
2021-10-26 12:02 ` Paul Menzel
2 siblings, 1 reply; 8+ messages in thread
From: Mark Yacoub @ 2021-10-13 18:12 UTC (permalink / raw)
To: dri-devel, amd-gfx, intel-gfx
Cc: seanpaul, harry.wentland, Mark Yacoub, Mark Yacoub, Leo Li,
Alex Deucher, Christian König, Pan, Xinhui, David Airlie,
Daniel Vetter, linux-kernel
From: Mark Yacoub <markyacoub@google.com>
[Why]
drm_atomic_helper_check_crtc now verifies both legacy and non-legacy LUT
sizes. There is no need to check it within amdgpu_dm_atomic_check.
[How]
Remove the local call to verify LUT sizes and use DRM Core function
instead.
Tested on ChromeOS Zork.
v1:
Remove amdgpu_dm_verify_lut_sizes everywhere.
Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 -
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 35 -------------------
3 files changed, 4 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f74663b6b046e..47f8de1cfc3a5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10244,6 +10244,10 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
}
}
#endif
+ ret = drm_atomic_helper_check_crtcs(state);
+ if (ret)
+ return ret;
+
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
@@ -10253,10 +10257,6 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
dm_old_crtc_state->dsc_force_changed == false)
continue;
- ret = amdgpu_dm_verify_lut_sizes(new_crtc_state);
- if (ret)
- goto fail;
-
if (!new_crtc_state->enable)
continue;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index fcb9c4a629c32..22730e5542092 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -617,7 +617,6 @@ void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
#define MAX_COLOR_LEGACY_LUT_ENTRIES 256
void amdgpu_dm_init_color_mod(void);
-int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state);
int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc);
int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
struct dc_plane_state *dc_plane_state);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index a022e5bb30a5c..319f8a8a89835 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -284,37 +284,6 @@ static int __set_input_tf(struct dc_transfer_func *func,
return res ? 0 : -ENOMEM;
}
-/**
- * Verifies that the Degamma and Gamma LUTs attached to the |crtc_state| are of
- * the expected size.
- * Returns 0 on success.
- */
-int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state)
-{
- const struct drm_color_lut *lut = NULL;
- uint32_t size = 0;
-
- lut = __extract_blob_lut(crtc_state->degamma_lut, &size);
- if (lut && size != MAX_COLOR_LUT_ENTRIES) {
- DRM_DEBUG_DRIVER(
- "Invalid Degamma LUT size. Should be %u but got %u.\n",
- MAX_COLOR_LUT_ENTRIES, size);
- return -EINVAL;
- }
-
- lut = __extract_blob_lut(crtc_state->gamma_lut, &size);
- if (lut && size != MAX_COLOR_LUT_ENTRIES &&
- size != MAX_COLOR_LEGACY_LUT_ENTRIES) {
- DRM_DEBUG_DRIVER(
- "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n",
- MAX_COLOR_LUT_ENTRIES, MAX_COLOR_LEGACY_LUT_ENTRIES,
- size);
- return -EINVAL;
- }
-
- return 0;
-}
-
/**
* amdgpu_dm_update_crtc_color_mgmt: Maps DRM color management to DC stream.
* @crtc: amdgpu_dm crtc state
@@ -348,10 +317,6 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
bool is_legacy;
int r;
- r = amdgpu_dm_verify_lut_sizes(&crtc->base);
- if (r)
- return r;
-
degamma_lut = __extract_blob_lut(crtc->base.degamma_lut, °amma_size);
regamma_lut = __extract_blob_lut(crtc->base.gamma_lut, ®amma_size);
--
2.33.0.882.g93a45727a2-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check
2021-10-13 18:12 ` [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check Mark Yacoub
@ 2021-10-14 23:42 ` Sean Paul
0 siblings, 0 replies; 8+ messages in thread
From: Sean Paul @ 2021-10-14 23:42 UTC (permalink / raw)
To: Mark Yacoub
Cc: dri-devel, amd-gfx, intel-gfx, harry.wentland, Mark Yacoub,
Leo Li, Alex Deucher, Christian König, Pan, Xinhui,
David Airlie, Daniel Vetter, linux-kernel
On Wed, Oct 13, 2021 at 2:12 PM Mark Yacoub <markyacoub@chromium.org> wrote:
>
> From: Mark Yacoub <markyacoub@google.com>
>
> [Why]
> drm_atomic_helper_check_crtc now verifies both legacy and non-legacy LUT
> sizes. There is no need to check it within amdgpu_dm_atomic_check.
>
> [How]
> Remove the local call to verify LUT sizes and use DRM Core function
> instead.
>
> Tested on ChromeOS Zork.
>
> v1:
> Remove amdgpu_dm_verify_lut_sizes everywhere.
>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 -
> .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 35 -------------------
> 3 files changed, 4 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index f74663b6b046e..47f8de1cfc3a5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -10244,6 +10244,10 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> }
> }
> #endif
> + ret = drm_atomic_helper_check_crtcs(state);
> + if (ret)
> + return ret;
> +
> for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
>
> @@ -10253,10 +10257,6 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> dm_old_crtc_state->dsc_force_changed == false)
> continue;
>
> - ret = amdgpu_dm_verify_lut_sizes(new_crtc_state);
> - if (ret)
> - goto fail;
> -
> if (!new_crtc_state->enable)
> continue;
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index fcb9c4a629c32..22730e5542092 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -617,7 +617,6 @@ void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
> #define MAX_COLOR_LEGACY_LUT_ENTRIES 256
>
> void amdgpu_dm_init_color_mod(void);
> -int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state);
> int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc);
> int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
> struct dc_plane_state *dc_plane_state);
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index a022e5bb30a5c..319f8a8a89835 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -284,37 +284,6 @@ static int __set_input_tf(struct dc_transfer_func *func,
> return res ? 0 : -ENOMEM;
> }
>
> -/**
> - * Verifies that the Degamma and Gamma LUTs attached to the |crtc_state| are of
> - * the expected size.
> - * Returns 0 on success.
> - */
> -int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state)
> -{
> - const struct drm_color_lut *lut = NULL;
> - uint32_t size = 0;
> -
> - lut = __extract_blob_lut(crtc_state->degamma_lut, &size);
> - if (lut && size != MAX_COLOR_LUT_ENTRIES) {
> - DRM_DEBUG_DRIVER(
> - "Invalid Degamma LUT size. Should be %u but got %u.\n",
> - MAX_COLOR_LUT_ENTRIES, size);
> - return -EINVAL;
> - }
> -
> - lut = __extract_blob_lut(crtc_state->gamma_lut, &size);
> - if (lut && size != MAX_COLOR_LUT_ENTRIES &&
> - size != MAX_COLOR_LEGACY_LUT_ENTRIES) {
> - DRM_DEBUG_DRIVER(
> - "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n",
> - MAX_COLOR_LUT_ENTRIES, MAX_COLOR_LEGACY_LUT_ENTRIES,
> - size);
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> /**
> * amdgpu_dm_update_crtc_color_mgmt: Maps DRM color management to DC stream.
> * @crtc: amdgpu_dm crtc state
> @@ -348,10 +317,6 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
> bool is_legacy;
> int r;
>
> - r = amdgpu_dm_verify_lut_sizes(&crtc->base);
> - if (r)
> - return r;
> -
> degamma_lut = __extract_blob_lut(crtc->base.degamma_lut, °amma_size);
> regamma_lut = __extract_blob_lut(crtc->base.gamma_lut, ®amma_size);
>
> --
> 2.33.0.882.g93a45727a2-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
2021-10-13 18:12 ` [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate Mark Yacoub
2021-10-13 18:12 ` [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check Mark Yacoub
@ 2021-10-26 1:26 ` Sean Paul
2021-10-26 19:25 ` Mark Yacoub
2021-10-26 12:02 ` Paul Menzel
2 siblings, 1 reply; 8+ messages in thread
From: Sean Paul @ 2021-10-26 1:26 UTC (permalink / raw)
To: Mark Yacoub
Cc: dri-devel, amd-gfx, intel-gfx, seanpaul, harry.wentland,
Mark Yacoub, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Matthias Brugger, linux-kernel, linux-arm-kernel,
linux-mediatek
On Wed, Oct 13, 2021 at 02:12:20PM -0400, Mark Yacoub wrote:
> From: Mark Yacoub <markyacoub@google.com>
>
> [Why]
> 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
> or Degamma props in the new CRTC state, allowing any invalid size to
> be passed on.
> 2. Each driver has its own LUT size, which could also be different for
> legacy users.
>
> [How]
> 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
> assigned by the driver when it's initializing its color and CTM
> management.
> 2. Create drm_atomic_helper_check_crtc which is called by
> drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
> they match the sizes in the new CRTC state.
> 3. Rename older lut checks that test for the color channels to indicate
> it's a channel check. It's not included in drm_atomic_helper_check_crtc
> as it's hardware specific and is to be called by the driver.
> 4. As the LUT size check now happens in drm_atomic_helper_check, remove
> the lut check in intel_color.c
I'd probably split the rename out from the crtc check since they're only
tangentially related.
>
> Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
> Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL)
>
> v1:
> 1. Fix typos
> 2. Remove the LUT size check from intel driver
> 3. Rename old LUT check to indicate it's a channel change
>
> Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++
> drivers/gpu/drm/drm_color_mgmt.c | 14 ++---
> drivers/gpu/drm/i915/display/intel_color.c | 14 ++---
> include/drm/drm_atomic_helper.h | 1 +
> include/drm/drm_color_mgmt.h | 7 +--
> include/drm/drm_crtc.h | 11 ++++
> 6 files changed, 89 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index bc3487964fb5e..5feb2ad0209c3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_helper_check_planes);
>
> +/**
> + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes
> + * @state: the driver state object
> + *
> + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
> + * state holds them.
> + *
> + * RETURNS:
> + * Zero for success or -errno
> + */
> +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *new_crtc_state;
> + int i;
> +
> + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> + if (new_crtc_state->color_mgmt_changed &&
> + new_crtc_state->gamma_lut) {
> + uint64_t supported_lut_size = crtc->gamma_lut_size;
> + uint32_t supported_legacy_lut_size = crtc->gamma_size;
> + uint32_t new_state_lut_size =
> + drm_color_lut_size(new_crtc_state->gamma_lut);
> +
> + if (new_state_lut_size != supported_lut_size &&
> + new_state_lut_size != supported_legacy_lut_size) {
> + drm_dbg_state(
> + state->dev,
> + "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n",
> + supported_lut_size,
> + supported_legacy_lut_size,
> + new_state_lut_size);
> + return -EINVAL;
> + }
> + }
> +
> + if (new_crtc_state->color_mgmt_changed &&
> + new_crtc_state->degamma_lut) {
> + uint32_t new_state_lut_size =
> + drm_color_lut_size(new_crtc_state->degamma_lut);
> + uint64_t supported_lut_size = crtc->degamma_lut_size;
> +
> + if (new_state_lut_size != supported_lut_size) {
> + drm_dbg_state(
> + state->dev,
> + "Invalid Degamma LUT size. Should be %u but got %u.\n",
> + supported_lut_size, new_state_lut_size);
> + return -EINVAL;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs);
> +
> /**
> * drm_atomic_helper_check - validate state object
> * @dev: DRM device
> @@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
> if (ret)
> return ret;
>
> + ret = drm_atomic_helper_check_crtcs(state);
> + if (ret)
> + return ret;
> +
> if (state->legacy_cursor_update)
> state->async_update = !drm_atomic_helper_async_check(dev, state);
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index bb14f488c8f6c..e5b820ce823bf 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> struct drm_mode_config *config = &dev->mode_config;
>
> if (degamma_lut_size) {
> + crtc->degamma_lut_size = degamma_lut_size;
> drm_object_attach_property(&crtc->base,
> config->degamma_lut_property, 0);
> drm_object_attach_property(&crtc->base,
> @@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> config->ctm_property, 0);
>
> if (gamma_lut_size) {
> + crtc->gamma_lut_size = gamma_lut_size;
> drm_object_attach_property(&crtc->base,
> config->gamma_lut_property, 0);
> drm_object_attach_property(&crtc->base,
> @@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> EXPORT_SYMBOL(drm_plane_create_color_properties);
>
> /**
> - * drm_color_lut_check - check validity of lookup table
> + * drm_color_lut_channels_check - check validity of the channels in the lookup table
> * @lut: property blob containing LUT to check
> * @tests: bitmask of tests to run
> *
> - * Helper to check whether a userspace-provided lookup table is valid and
> - * satisfies hardware requirements. Drivers pass a bitmask indicating which of
> - * the tests in &drm_color_lut_tests should be performed.
> + * Helper to check whether each color channel of userspace-provided lookup table is valid and
> + * satisfies hardware requirements. Drivers pass a bitmask indicating which of in
> + * &drm_color_lut_channels_tests should be performed.
> *
> * Returns 0 on success, -EINVAL on failure.
> */
> -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
> +int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests)
> {
> const struct drm_color_lut *entry;
> int i;
> @@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
>
> return 0;
> }
> -EXPORT_SYMBOL(drm_color_lut_check);
> +EXPORT_SYMBOL(drm_color_lut_channels_check);
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index dab892d2251ba..a308fe52746ac 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
> const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
> int gamma_length, degamma_length;
> - u32 gamma_tests, degamma_tests;
> + u32 gamma_channels_tests, degamma_channels_tests;
>
> /* Always allow legacy gamma LUT with no further checking. */
> if (crtc_state_is_legacy_gamma(crtc_state))
> @@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
>
> degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> - degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> - gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
> + degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> + gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
>
> - if (check_lut_size(degamma_lut, degamma_length) ||
> - check_lut_size(gamma_lut, gamma_length))
> - return -EINVAL;
Can you remove check_lut_size() now?
> -
> - if (drm_color_lut_check(degamma_lut, degamma_tests) ||
> - drm_color_lut_check(gamma_lut, gamma_tests))
> + if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) ||
> + drm_color_lut_channels_check(gamma_lut, gamma_channels_tests))
> return -EINVAL;
>
> return 0;
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 4045e2507e11c..a22d32a7a8719 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -38,6 +38,7 @@ struct drm_atomic_state;
> struct drm_private_obj;
> struct drm_private_state;
>
> +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state);
> int drm_atomic_helper_check_modeset(struct drm_device *dev,
> struct drm_atomic_state *state);
> int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 81c298488b0c8..cb1bf361ad3e3 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> enum drm_color_range default_range);
>
> /**
> - * enum drm_color_lut_tests - hw-specific LUT tests to perform
> + * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform
> *
> * The drm_color_lut_check() function takes a bitmask of the values here to
> * determine which tests to apply to a userspace-provided LUT.
> */
> -enum drm_color_lut_tests {
> +enum drm_color_lut_channels_tests {
> /**
> * @DRM_COLOR_LUT_EQUAL_CHANNELS:
> *
> @@ -119,5 +119,6 @@ enum drm_color_lut_tests {
> DRM_COLOR_LUT_NON_DECREASING = BIT(1),
> };
>
> -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests);
> +int drm_color_lut_channels_check(const struct drm_property_blob *lut,
> + u32 tests);
> #endif
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 2deb15d7e1610..cabd3ef1a6e32 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1072,6 +1072,17 @@ struct drm_crtc {
> /** @funcs: CRTC control functions */
> const struct drm_crtc_funcs *funcs;
>
> + /**
> + * @degamma_lut_size: Size of degamma LUT.
> + */
> + uint32_t degamma_lut_size;
> +
> + /**
> + * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as
> + * X, which doesn't support large lut sizes.
> + */
> + uint32_t gamma_lut_size;
> +
> /**
> * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up
> * by calling drm_mode_crtc_set_gamma_size().
> --
> 2.33.0.882.g93a45727a2-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
2021-10-13 18:12 ` [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate Mark Yacoub
2021-10-13 18:12 ` [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check Mark Yacoub
2021-10-26 1:26 ` [Intel-gfx] [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate Sean Paul
@ 2021-10-26 12:02 ` Paul Menzel
2021-10-26 19:24 ` Mark Yacoub
2 siblings, 1 reply; 8+ messages in thread
From: Paul Menzel @ 2021-10-26 12:02 UTC (permalink / raw)
To: Mark Yacoub
Cc: seanpaul, harry.wentland, Mark Yacoub, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Matthias Brugger,
linux-kernel, linux-arm-kernel, linux-mediatek, dri-devel,
amd-gfx, intel-gfx
Dear Mark,
Thank you for your patch.
On 13.10.21 20:12, Mark Yacoub wrote:
> From: Mark Yacoub <markyacoub@google.com>
>
> [Why]
> 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
> or Degamma props in the new CRTC state, allowing any invalid size to
> be passed on.
> 2. Each driver has its own LUT size, which could also be different for
> legacy users.
How can the problem be reproduced?
> [How]
> 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
> assigned by the driver when it's initializing its color and CTM
> management.
> 2. Create drm_atomic_helper_check_crtc which is called by
> drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
> they match the sizes in the new CRTC state.
> 3. Rename older lut checks that test for the color channels to indicate
> it's a channel check. It's not included in drm_atomic_helper_check_crtc
> as it's hardware specific and is to be called by the driver.
> 4. As the LUT size check now happens in drm_atomic_helper_check, remove
> the lut check in intel_color.c
>
> Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
If I am not mistaken, the Fixes tag is used for commits I believe. Maybe
use Resolves or something similar?
> Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL)
Please add a space before the (.
How did you test this?
> v1:
> 1. Fix typos
> 2. Remove the LUT size check from intel driver
> 3. Rename old LUT check to indicate it's a channel change
>
> Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++
> drivers/gpu/drm/drm_color_mgmt.c | 14 ++---
> drivers/gpu/drm/i915/display/intel_color.c | 14 ++---
> include/drm/drm_atomic_helper.h | 1 +
> include/drm/drm_color_mgmt.h | 7 +--
> include/drm/drm_crtc.h | 11 ++++
> 6 files changed, 89 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index bc3487964fb5e..5feb2ad0209c3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_helper_check_planes);
>
> +/**
> + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes
> + * @state: the driver state object
> + *
> + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
> + * state holds them.
> + *
> + * RETURNS:
> + * Zero for success or -errno
> + */
> +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *new_crtc_state;
> + int i;
> +
> + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> + if (new_crtc_state->color_mgmt_changed &&
> + new_crtc_state->gamma_lut) {
> + uint64_t supported_lut_size = crtc->gamma_lut_size;
> + uint32_t supported_legacy_lut_size = crtc->gamma_size;
> + uint32_t new_state_lut_size =
> + drm_color_lut_size(new_crtc_state->gamma_lut);
> +
> + if (new_state_lut_size != supported_lut_size &&
> + new_state_lut_size != supported_legacy_lut_size) {
> + drm_dbg_state(
> + state->dev,
> + "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n",
> + supported_lut_size,
> + supported_legacy_lut_size,
> + new_state_lut_size);
> + return -EINVAL;
> + }
> + }
> +
> + if (new_crtc_state->color_mgmt_changed &&
> + new_crtc_state->degamma_lut) {
> + uint32_t new_state_lut_size =
> + drm_color_lut_size(new_crtc_state->degamma_lut);
> + uint64_t supported_lut_size = crtc->degamma_lut_size;
> +
> + if (new_state_lut_size != supported_lut_size) {
> + drm_dbg_state(
> + state->dev,
> + "Invalid Degamma LUT size. Should be %u but got %u.\n",
> + supported_lut_size, new_state_lut_size);
> + return -EINVAL;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs);
> +
> /**
> * drm_atomic_helper_check - validate state object
> * @dev: DRM device
> @@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
> if (ret)
> return ret;
>
> + ret = drm_atomic_helper_check_crtcs(state);
> + if (ret)
> + return ret;
> +
> if (state->legacy_cursor_update)
> state->async_update = !drm_atomic_helper_async_check(dev, state);
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index bb14f488c8f6c..e5b820ce823bf 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> struct drm_mode_config *config = &dev->mode_config;
>
> if (degamma_lut_size) {
> + crtc->degamma_lut_size = degamma_lut_size;
> drm_object_attach_property(&crtc->base,
> config->degamma_lut_property, 0);
> drm_object_attach_property(&crtc->base,
> @@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> config->ctm_property, 0);
>
> if (gamma_lut_size) {
> + crtc->gamma_lut_size = gamma_lut_size;
> drm_object_attach_property(&crtc->base,
> config->gamma_lut_property, 0);
> drm_object_attach_property(&crtc->base,
> @@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> EXPORT_SYMBOL(drm_plane_create_color_properties);
>
> /**
> - * drm_color_lut_check - check validity of lookup table
> + * drm_color_lut_channels_check - check validity of the channels in the lookup table
> * @lut: property blob containing LUT to check
> * @tests: bitmask of tests to run
> *
> - * Helper to check whether a userspace-provided lookup table is valid and
> - * satisfies hardware requirements. Drivers pass a bitmask indicating which of
> - * the tests in &drm_color_lut_tests should be performed.
> + * Helper to check whether each color channel of userspace-provided lookup table is valid and
> + * satisfies hardware requirements. Drivers pass a bitmask indicating which of in
> + * &drm_color_lut_channels_tests should be performed.
> *
> * Returns 0 on success, -EINVAL on failure.
> */
> -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
> +int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests)
> {
> const struct drm_color_lut *entry;
> int i;
> @@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
>
> return 0;
> }
> -EXPORT_SYMBOL(drm_color_lut_check);
> +EXPORT_SYMBOL(drm_color_lut_channels_check);
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index dab892d2251ba..a308fe52746ac 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
> const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
> int gamma_length, degamma_length;
> - u32 gamma_tests, degamma_tests;
> + u32 gamma_channels_tests, degamma_channels_tests;
>
> /* Always allow legacy gamma LUT with no further checking. */
> if (crtc_state_is_legacy_gamma(crtc_state))
> @@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
>
> degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> - degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> - gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
> + degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> + gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
>
> - if (check_lut_size(degamma_lut, degamma_length) ||
> - check_lut_size(gamma_lut, gamma_length))
> - return -EINVAL;
> -
> - if (drm_color_lut_check(degamma_lut, degamma_tests) ||
> - drm_color_lut_check(gamma_lut, gamma_tests))
> + if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) ||
> + drm_color_lut_channels_check(gamma_lut, gamma_channels_tests))
> return -EINVAL;
>
> return 0;
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 4045e2507e11c..a22d32a7a8719 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -38,6 +38,7 @@ struct drm_atomic_state;
> struct drm_private_obj;
> struct drm_private_state;
>
> +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state);
> int drm_atomic_helper_check_modeset(struct drm_device *dev,
> struct drm_atomic_state *state);
> int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 81c298488b0c8..cb1bf361ad3e3 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> enum drm_color_range default_range);
>
> /**
> - * enum drm_color_lut_tests - hw-specific LUT tests to perform
> + * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform
> *
> * The drm_color_lut_check() function takes a bitmask of the values here to
> * determine which tests to apply to a userspace-provided LUT.
> */
> -enum drm_color_lut_tests {
> +enum drm_color_lut_channels_tests {
> /**
> * @DRM_COLOR_LUT_EQUAL_CHANNELS:
> *
> @@ -119,5 +119,6 @@ enum drm_color_lut_tests {
> DRM_COLOR_LUT_NON_DECREASING = BIT(1),
> };
>
> -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests);
> +int drm_color_lut_channels_check(const struct drm_property_blob *lut,
> + u32 tests);
> #endif
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 2deb15d7e1610..cabd3ef1a6e32 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1072,6 +1072,17 @@ struct drm_crtc {
> /** @funcs: CRTC control functions */
> const struct drm_crtc_funcs *funcs;
>
> + /**
> + * @degamma_lut_size: Size of degamma LUT.
> + */
> + uint32_t degamma_lut_size;
> +
> + /**
> + * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as
> + * X, which doesn't support large lut sizes.
> + */
> + uint32_t gamma_lut_size;
> +
> /**
> * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up
> * by calling drm_mode_crtc_set_gamma_size().
>
Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
2021-10-26 12:02 ` Paul Menzel
@ 2021-10-26 19:24 ` Mark Yacoub
2021-10-26 19:26 ` Mark Yacoub
0 siblings, 1 reply; 8+ messages in thread
From: Mark Yacoub @ 2021-10-26 19:24 UTC (permalink / raw)
To: Paul Menzel
Cc: seanpaul, harry.wentland, Mark Yacoub, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Matthias Brugger,
linux-kernel, linux-arm-kernel, linux-mediatek, dri-devel,
amd-gfx, intel-gfx
On Tue, Oct 26, 2021 at 8:02 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Mark,
>
>
> Thank you for your patch.
>
> On 13.10.21 20:12, Mark Yacoub wrote:
> > From: Mark Yacoub <markyacoub@google.com>
> >
> > [Why]
> > 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
> > or Degamma props in the new CRTC state, allowing any invalid size to
> > be passed on.
> > 2. Each driver has its own LUT size, which could also be different for
> > legacy users.
>
> How can the problem be reproduced?
It was caught using igt@kms_color@pipe-A-invalid-gamma-lut-sizes.
it validates that drivers will only LUTs of their expected size not
any random (smaller or larger) number.
>
> > [How]
> > 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
> > assigned by the driver when it's initializing its color and CTM
> > management.
> > 2. Create drm_atomic_helper_check_crtc which is called by
> > drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
> > they match the sizes in the new CRTC state.
> > 3. Rename older lut checks that test for the color channels to indicate
> > it's a channel check. It's not included in drm_atomic_helper_check_crtc
> > as it's hardware specific and is to be called by the driver.
> > 4. As the LUT size check now happens in drm_atomic_helper_check, remove
> > the lut check in intel_color.c
> >
> > Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
>
> If I am not mistaken, the Fixes tag is used for commits I believe. Maybe
> use Resolves or something similar?
fixed!
>
> > Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL)
>
> Please add a space before the (.
>
> How did you test this?
smoke test on both MTK and TGL devices along with running
igt@kms_color on both devices.
>
> > v1:
> > 1. Fix typos
> > 2. Remove the LUT size check from intel driver
> > 3. Rename old LUT check to indicate it's a channel change
> >
> > Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++
> > drivers/gpu/drm/drm_color_mgmt.c | 14 ++---
> > drivers/gpu/drm/i915/display/intel_color.c | 14 ++---
> > include/drm/drm_atomic_helper.h | 1 +
> > include/drm/drm_color_mgmt.h | 7 +--
> > include/drm/drm_crtc.h | 11 ++++
> > 6 files changed, 89 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index bc3487964fb5e..5feb2ad0209c3 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_check_planes);
> >
> > +/**
> > + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes
> > + * @state: the driver state object
> > + *
> > + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
> > + * state holds them.
> > + *
> > + * RETURNS:
> > + * Zero for success or -errno
> > + */
> > +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state)
> > +{
> > + struct drm_crtc *crtc;
> > + struct drm_crtc_state *new_crtc_state;
> > + int i;
> > +
> > + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> > + if (new_crtc_state->color_mgmt_changed &&
> > + new_crtc_state->gamma_lut) {
> > + uint64_t supported_lut_size = crtc->gamma_lut_size;
> > + uint32_t supported_legacy_lut_size = crtc->gamma_size;
> > + uint32_t new_state_lut_size =
> > + drm_color_lut_size(new_crtc_state->gamma_lut);
> > +
> > + if (new_state_lut_size != supported_lut_size &&
> > + new_state_lut_size != supported_legacy_lut_size) {
> > + drm_dbg_state(
> > + state->dev,
> > + "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n",
> > + supported_lut_size,
> > + supported_legacy_lut_size,
> > + new_state_lut_size);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + if (new_crtc_state->color_mgmt_changed &&
> > + new_crtc_state->degamma_lut) {
> > + uint32_t new_state_lut_size =
> > + drm_color_lut_size(new_crtc_state->degamma_lut);
> > + uint64_t supported_lut_size = crtc->degamma_lut_size;
> > +
> > + if (new_state_lut_size != supported_lut_size) {
> > + drm_dbg_state(
> > + state->dev,
> > + "Invalid Degamma LUT size. Should be %u but got %u.\n",
> > + supported_lut_size, new_state_lut_size);
> > + return -EINVAL;
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs);
> > +
> > /**
> > * drm_atomic_helper_check - validate state object
> > * @dev: DRM device
> > @@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
> > if (ret)
> > return ret;
> >
> > + ret = drm_atomic_helper_check_crtcs(state);
> > + if (ret)
> > + return ret;
> > +
> > if (state->legacy_cursor_update)
> > state->async_update = !drm_atomic_helper_async_check(dev, state);
> >
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index bb14f488c8f6c..e5b820ce823bf 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > struct drm_mode_config *config = &dev->mode_config;
> >
> > if (degamma_lut_size) {
> > + crtc->degamma_lut_size = degamma_lut_size;
> > drm_object_attach_property(&crtc->base,
> > config->degamma_lut_property, 0);
> > drm_object_attach_property(&crtc->base,
> > @@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > config->ctm_property, 0);
> >
> > if (gamma_lut_size) {
> > + crtc->gamma_lut_size = gamma_lut_size;
> > drm_object_attach_property(&crtc->base,
> > config->gamma_lut_property, 0);
> > drm_object_attach_property(&crtc->base,
> > @@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> > EXPORT_SYMBOL(drm_plane_create_color_properties);
> >
> > /**
> > - * drm_color_lut_check - check validity of lookup table
> > + * drm_color_lut_channels_check - check validity of the channels in the lookup table
> > * @lut: property blob containing LUT to check
> > * @tests: bitmask of tests to run
> > *
> > - * Helper to check whether a userspace-provided lookup table is valid and
> > - * satisfies hardware requirements. Drivers pass a bitmask indicating which of
> > - * the tests in &drm_color_lut_tests should be performed.
> > + * Helper to check whether each color channel of userspace-provided lookup table is valid and
> > + * satisfies hardware requirements. Drivers pass a bitmask indicating which of in
> > + * &drm_color_lut_channels_tests should be performed.
> > *
> > * Returns 0 on success, -EINVAL on failure.
> > */
> > -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
> > +int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests)
> > {
> > const struct drm_color_lut *entry;
> > int i;
> > @@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
> >
> > return 0;
> > }
> > -EXPORT_SYMBOL(drm_color_lut_check);
> > +EXPORT_SYMBOL(drm_color_lut_channels_check);
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> > index dab892d2251ba..a308fe52746ac 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
> > const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> > const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
> > int gamma_length, degamma_length;
> > - u32 gamma_tests, degamma_tests;
> > + u32 gamma_channels_tests, degamma_channels_tests;
> >
> > /* Always allow legacy gamma LUT with no further checking. */
> > if (crtc_state_is_legacy_gamma(crtc_state))
> > @@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
> >
> > degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> > gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> > - degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> > - gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
> > + degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> > + gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
> >
> > - if (check_lut_size(degamma_lut, degamma_length) ||
> > - check_lut_size(gamma_lut, gamma_length))
> > - return -EINVAL;
> > -
> > - if (drm_color_lut_check(degamma_lut, degamma_tests) ||
> > - drm_color_lut_check(gamma_lut, gamma_tests))
> > + if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) ||
> > + drm_color_lut_channels_check(gamma_lut, gamma_channels_tests))
> > return -EINVAL;
> >
> > return 0;
> > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> > index 4045e2507e11c..a22d32a7a8719 100644
> > --- a/include/drm/drm_atomic_helper.h
> > +++ b/include/drm/drm_atomic_helper.h
> > @@ -38,6 +38,7 @@ struct drm_atomic_state;
> > struct drm_private_obj;
> > struct drm_private_state;
> >
> > +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state);
> > int drm_atomic_helper_check_modeset(struct drm_device *dev,
> > struct drm_atomic_state *state);
> > int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index 81c298488b0c8..cb1bf361ad3e3 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> > enum drm_color_range default_range);
> >
> > /**
> > - * enum drm_color_lut_tests - hw-specific LUT tests to perform
> > + * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform
> > *
> > * The drm_color_lut_check() function takes a bitmask of the values here to
> > * determine which tests to apply to a userspace-provided LUT.
> > */
> > -enum drm_color_lut_tests {
> > +enum drm_color_lut_channels_tests {
> > /**
> > * @DRM_COLOR_LUT_EQUAL_CHANNELS:
> > *
> > @@ -119,5 +119,6 @@ enum drm_color_lut_tests {
> > DRM_COLOR_LUT_NON_DECREASING = BIT(1),
> > };
> >
> > -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests);
> > +int drm_color_lut_channels_check(const struct drm_property_blob *lut,
> > + u32 tests);
> > #endif
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 2deb15d7e1610..cabd3ef1a6e32 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -1072,6 +1072,17 @@ struct drm_crtc {
> > /** @funcs: CRTC control functions */
> > const struct drm_crtc_funcs *funcs;
> >
> > + /**
> > + * @degamma_lut_size: Size of degamma LUT.
> > + */
> > + uint32_t degamma_lut_size;
> > +
> > + /**
> > + * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as
> > + * X, which doesn't support large lut sizes.
> > + */
> > + uint32_t gamma_lut_size;
> > +
> > /**
> > * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up
> > * by calling drm_mode_crtc_set_gamma_size().
> >
>
> Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
>
> Kind regards,
>
> Paul
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
2021-10-26 1:26 ` [Intel-gfx] [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate Sean Paul
@ 2021-10-26 19:25 ` Mark Yacoub
0 siblings, 0 replies; 8+ messages in thread
From: Mark Yacoub @ 2021-10-26 19:25 UTC (permalink / raw)
To: Sean Paul
Cc: dri-devel, amd-gfx, intel-gfx, seanpaul, harry.wentland,
Mark Yacoub, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Matthias Brugger, linux-kernel, linux-arm-kernel,
linux-mediatek
On Mon, Oct 25, 2021 at 9:26 PM Sean Paul <sean@poorly.run> wrote:
>
> On Wed, Oct 13, 2021 at 02:12:20PM -0400, Mark Yacoub wrote:
> > From: Mark Yacoub <markyacoub@google.com>
> >
> > [Why]
> > 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
> > or Degamma props in the new CRTC state, allowing any invalid size to
> > be passed on.
> > 2. Each driver has its own LUT size, which could also be different for
> > legacy users.
> >
> > [How]
> > 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
> > assigned by the driver when it's initializing its color and CTM
> > management.
> > 2. Create drm_atomic_helper_check_crtc which is called by
> > drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
> > they match the sizes in the new CRTC state.
> > 3. Rename older lut checks that test for the color channels to indicate
> > it's a channel check. It's not included in drm_atomic_helper_check_crtc
> > as it's hardware specific and is to be called by the driver.
> > 4. As the LUT size check now happens in drm_atomic_helper_check, remove
> > the lut check in intel_color.c
>
> I'd probably split the rename out from the crtc check since they're only
> tangentially related.
done.
>
> >
> > Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
> > Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL)
> >
> > v1:
> > 1. Fix typos
> > 2. Remove the LUT size check from intel driver
> > 3. Rename old LUT check to indicate it's a channel change
> >
> > Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++
> > drivers/gpu/drm/drm_color_mgmt.c | 14 ++---
> > drivers/gpu/drm/i915/display/intel_color.c | 14 ++---
> > include/drm/drm_atomic_helper.h | 1 +
> > include/drm/drm_color_mgmt.h | 7 +--
> > include/drm/drm_crtc.h | 11 ++++
> > 6 files changed, 89 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index bc3487964fb5e..5feb2ad0209c3 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_check_planes);
> >
> > +/**
> > + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes
> > + * @state: the driver state object
> > + *
> > + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
> > + * state holds them.
> > + *
> > + * RETURNS:
> > + * Zero for success or -errno
> > + */
> > +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state)
> > +{
> > + struct drm_crtc *crtc;
> > + struct drm_crtc_state *new_crtc_state;
> > + int i;
> > +
> > + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> > + if (new_crtc_state->color_mgmt_changed &&
> > + new_crtc_state->gamma_lut) {
> > + uint64_t supported_lut_size = crtc->gamma_lut_size;
> > + uint32_t supported_legacy_lut_size = crtc->gamma_size;
> > + uint32_t new_state_lut_size =
> > + drm_color_lut_size(new_crtc_state->gamma_lut);
> > +
> > + if (new_state_lut_size != supported_lut_size &&
> > + new_state_lut_size != supported_legacy_lut_size) {
> > + drm_dbg_state(
> > + state->dev,
> > + "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n",
> > + supported_lut_size,
> > + supported_legacy_lut_size,
> > + new_state_lut_size);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + if (new_crtc_state->color_mgmt_changed &&
> > + new_crtc_state->degamma_lut) {
> > + uint32_t new_state_lut_size =
> > + drm_color_lut_size(new_crtc_state->degamma_lut);
> > + uint64_t supported_lut_size = crtc->degamma_lut_size;
> > +
> > + if (new_state_lut_size != supported_lut_size) {
> > + drm_dbg_state(
> > + state->dev,
> > + "Invalid Degamma LUT size. Should be %u but got %u.\n",
> > + supported_lut_size, new_state_lut_size);
> > + return -EINVAL;
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs);
> > +
> > /**
> > * drm_atomic_helper_check - validate state object
> > * @dev: DRM device
> > @@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
> > if (ret)
> > return ret;
> >
> > + ret = drm_atomic_helper_check_crtcs(state);
> > + if (ret)
> > + return ret;
> > +
> > if (state->legacy_cursor_update)
> > state->async_update = !drm_atomic_helper_async_check(dev, state);
> >
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index bb14f488c8f6c..e5b820ce823bf 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > struct drm_mode_config *config = &dev->mode_config;
> >
> > if (degamma_lut_size) {
> > + crtc->degamma_lut_size = degamma_lut_size;
> > drm_object_attach_property(&crtc->base,
> > config->degamma_lut_property, 0);
> > drm_object_attach_property(&crtc->base,
> > @@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > config->ctm_property, 0);
> >
> > if (gamma_lut_size) {
> > + crtc->gamma_lut_size = gamma_lut_size;
> > drm_object_attach_property(&crtc->base,
> > config->gamma_lut_property, 0);
> > drm_object_attach_property(&crtc->base,
> > @@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> > EXPORT_SYMBOL(drm_plane_create_color_properties);
> >
> > /**
> > - * drm_color_lut_check - check validity of lookup table
> > + * drm_color_lut_channels_check - check validity of the channels in the lookup table
> > * @lut: property blob containing LUT to check
> > * @tests: bitmask of tests to run
> > *
> > - * Helper to check whether a userspace-provided lookup table is valid and
> > - * satisfies hardware requirements. Drivers pass a bitmask indicating which of
> > - * the tests in &drm_color_lut_tests should be performed.
> > + * Helper to check whether each color channel of userspace-provided lookup table is valid and
> > + * satisfies hardware requirements. Drivers pass a bitmask indicating which of in
> > + * &drm_color_lut_channels_tests should be performed.
> > *
> > * Returns 0 on success, -EINVAL on failure.
> > */
> > -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
> > +int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests)
> > {
> > const struct drm_color_lut *entry;
> > int i;
> > @@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
> >
> > return 0;
> > }
> > -EXPORT_SYMBOL(drm_color_lut_check);
> > +EXPORT_SYMBOL(drm_color_lut_channels_check);
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> > index dab892d2251ba..a308fe52746ac 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
> > const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> > const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
> > int gamma_length, degamma_length;
> > - u32 gamma_tests, degamma_tests;
> > + u32 gamma_channels_tests, degamma_channels_tests;
> >
> > /* Always allow legacy gamma LUT with no further checking. */
> > if (crtc_state_is_legacy_gamma(crtc_state))
> > @@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
> >
> > degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> > gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> > - degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> > - gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
> > + degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> > + gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
> >
> > - if (check_lut_size(degamma_lut, degamma_length) ||
> > - check_lut_size(gamma_lut, gamma_length))
> > - return -EINVAL;
>
> Can you remove check_lut_size() now?
>
replace by drm_ check_lut_size
still gotta keep it cause you reach this both using set prop, not
necessarily through an atomic commit only.
> > -
> > - if (drm_color_lut_check(degamma_lut, degamma_tests) ||
> > - drm_color_lut_check(gamma_lut, gamma_tests))
> > + if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) ||
> > + drm_color_lut_channels_check(gamma_lut, gamma_channels_tests))
> > return -EINVAL;
> >
> > return 0;
> > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> > index 4045e2507e11c..a22d32a7a8719 100644
> > --- a/include/drm/drm_atomic_helper.h
> > +++ b/include/drm/drm_atomic_helper.h
> > @@ -38,6 +38,7 @@ struct drm_atomic_state;
> > struct drm_private_obj;
> > struct drm_private_state;
> >
> > +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state);
> > int drm_atomic_helper_check_modeset(struct drm_device *dev,
> > struct drm_atomic_state *state);
> > int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index 81c298488b0c8..cb1bf361ad3e3 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> > enum drm_color_range default_range);
> >
> > /**
> > - * enum drm_color_lut_tests - hw-specific LUT tests to perform
> > + * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform
> > *
> > * The drm_color_lut_check() function takes a bitmask of the values here to
> > * determine which tests to apply to a userspace-provided LUT.
> > */
> > -enum drm_color_lut_tests {
> > +enum drm_color_lut_channels_tests {
> > /**
> > * @DRM_COLOR_LUT_EQUAL_CHANNELS:
> > *
> > @@ -119,5 +119,6 @@ enum drm_color_lut_tests {
> > DRM_COLOR_LUT_NON_DECREASING = BIT(1),
> > };
> >
> > -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests);
> > +int drm_color_lut_channels_check(const struct drm_property_blob *lut,
> > + u32 tests);
> > #endif
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 2deb15d7e1610..cabd3ef1a6e32 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -1072,6 +1072,17 @@ struct drm_crtc {
> > /** @funcs: CRTC control functions */
> > const struct drm_crtc_funcs *funcs;
> >
> > + /**
> > + * @degamma_lut_size: Size of degamma LUT.
> > + */
> > + uint32_t degamma_lut_size;
> > +
> > + /**
> > + * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as
> > + * X, which doesn't support large lut sizes.
> > + */
> > + uint32_t gamma_lut_size;
> > +
> > /**
> > * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up
> > * by calling drm_mode_crtc_set_gamma_size().
> > --
> > 2.33.0.882.g93a45727a2-goog
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
2021-10-26 19:24 ` Mark Yacoub
@ 2021-10-26 19:26 ` Mark Yacoub
0 siblings, 0 replies; 8+ messages in thread
From: Mark Yacoub @ 2021-10-26 19:26 UTC (permalink / raw)
To: Paul Menzel
Cc: seanpaul, harry.wentland, Mark Yacoub, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Matthias Brugger,
linux-kernel, linux-arm-kernel, linux-mediatek, dri-devel,
amd-gfx, intel-gfx
new patch: https://patchwork.freedesktop.org/series/96314/
On Tue, Oct 26, 2021 at 3:24 PM Mark Yacoub <markyacoub@chromium.org> wrote:
>
> On Tue, Oct 26, 2021 at 8:02 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> >
> > Dear Mark,
> >
> >
> > Thank you for your patch.
> >
> > On 13.10.21 20:12, Mark Yacoub wrote:
> > > From: Mark Yacoub <markyacoub@google.com>
> > >
> > > [Why]
> > > 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
> > > or Degamma props in the new CRTC state, allowing any invalid size to
> > > be passed on.
> > > 2. Each driver has its own LUT size, which could also be different for
> > > legacy users.
> >
> > How can the problem be reproduced?
> It was caught using igt@kms_color@pipe-A-invalid-gamma-lut-sizes.
> it validates that drivers will only LUTs of their expected size not
> any random (smaller or larger) number.
> >
> > > [How]
> > > 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
> > > assigned by the driver when it's initializing its color and CTM
> > > management.
> > > 2. Create drm_atomic_helper_check_crtc which is called by
> > > drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
> > > they match the sizes in the new CRTC state.
> > > 3. Rename older lut checks that test for the color channels to indicate
> > > it's a channel check. It's not included in drm_atomic_helper_check_crtc
> > > as it's hardware specific and is to be called by the driver.
> > > 4. As the LUT size check now happens in drm_atomic_helper_check, remove
> > > the lut check in intel_color.c
> > >
> > > Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
> >
> > If I am not mistaken, the Fixes tag is used for commits I believe. Maybe
> > use Resolves or something similar?
> fixed!
> >
> > > Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL)
> >
> > Please add a space before the (.
my apologies i missed this. I'll update it if another version is
needed or if the committer can do it for me when they apply it.
> >
> > How did you test this?
> smoke test on both MTK and TGL devices along with running
> igt@kms_color on both devices.
> >
> > > v1:
> > > 1. Fix typos
> > > 2. Remove the LUT size check from intel driver
> > > 3. Rename old LUT check to indicate it's a channel change
> > >
> > > Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> > > ---
> > > drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++
> > > drivers/gpu/drm/drm_color_mgmt.c | 14 ++---
> > > drivers/gpu/drm/i915/display/intel_color.c | 14 ++---
> > > include/drm/drm_atomic_helper.h | 1 +
> > > include/drm/drm_color_mgmt.h | 7 +--
> > > include/drm/drm_crtc.h | 11 ++++
> > > 6 files changed, 89 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > index bc3487964fb5e..5feb2ad0209c3 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> > > }
> > > EXPORT_SYMBOL(drm_atomic_helper_check_planes);
> > >
> > > +/**
> > > + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes
> > > + * @state: the driver state object
> > > + *
> > > + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
> > > + * state holds them.
> > > + *
> > > + * RETURNS:
> > > + * Zero for success or -errno
> > > + */
> > > +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state)
> > > +{
> > > + struct drm_crtc *crtc;
> > > + struct drm_crtc_state *new_crtc_state;
> > > + int i;
> > > +
> > > + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> > > + if (new_crtc_state->color_mgmt_changed &&
> > > + new_crtc_state->gamma_lut) {
> > > + uint64_t supported_lut_size = crtc->gamma_lut_size;
> > > + uint32_t supported_legacy_lut_size = crtc->gamma_size;
> > > + uint32_t new_state_lut_size =
> > > + drm_color_lut_size(new_crtc_state->gamma_lut);
> > > +
> > > + if (new_state_lut_size != supported_lut_size &&
> > > + new_state_lut_size != supported_legacy_lut_size) {
> > > + drm_dbg_state(
> > > + state->dev,
> > > + "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n",
> > > + supported_lut_size,
> > > + supported_legacy_lut_size,
> > > + new_state_lut_size);
> > > + return -EINVAL;
> > > + }
> > > + }
> > > +
> > > + if (new_crtc_state->color_mgmt_changed &&
> > > + new_crtc_state->degamma_lut) {
> > > + uint32_t new_state_lut_size =
> > > + drm_color_lut_size(new_crtc_state->degamma_lut);
> > > + uint64_t supported_lut_size = crtc->degamma_lut_size;
> > > +
> > > + if (new_state_lut_size != supported_lut_size) {
> > > + drm_dbg_state(
> > > + state->dev,
> > > + "Invalid Degamma LUT size. Should be %u but got %u.\n",
> > > + supported_lut_size, new_state_lut_size);
> > > + return -EINVAL;
> > > + }
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs);
> > > +
> > > /**
> > > * drm_atomic_helper_check - validate state object
> > > * @dev: DRM device
> > > @@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
> > > if (ret)
> > > return ret;
> > >
> > > + ret = drm_atomic_helper_check_crtcs(state);
> > > + if (ret)
> > > + return ret;
> > > +
> > > if (state->legacy_cursor_update)
> > > state->async_update = !drm_atomic_helper_async_check(dev, state);
> > >
> > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > > index bb14f488c8f6c..e5b820ce823bf 100644
> > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > > struct drm_mode_config *config = &dev->mode_config;
> > >
> > > if (degamma_lut_size) {
> > > + crtc->degamma_lut_size = degamma_lut_size;
> > > drm_object_attach_property(&crtc->base,
> > > config->degamma_lut_property, 0);
> > > drm_object_attach_property(&crtc->base,
> > > @@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > > config->ctm_property, 0);
> > >
> > > if (gamma_lut_size) {
> > > + crtc->gamma_lut_size = gamma_lut_size;
> > > drm_object_attach_property(&crtc->base,
> > > config->gamma_lut_property, 0);
> > > drm_object_attach_property(&crtc->base,
> > > @@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> > > EXPORT_SYMBOL(drm_plane_create_color_properties);
> > >
> > > /**
> > > - * drm_color_lut_check - check validity of lookup table
> > > + * drm_color_lut_channels_check - check validity of the channels in the lookup table
> > > * @lut: property blob containing LUT to check
> > > * @tests: bitmask of tests to run
> > > *
> > > - * Helper to check whether a userspace-provided lookup table is valid and
> > > - * satisfies hardware requirements. Drivers pass a bitmask indicating which of
> > > - * the tests in &drm_color_lut_tests should be performed.
> > > + * Helper to check whether each color channel of userspace-provided lookup table is valid and
> > > + * satisfies hardware requirements. Drivers pass a bitmask indicating which of in
> > > + * &drm_color_lut_channels_tests should be performed.
> > > *
> > > * Returns 0 on success, -EINVAL on failure.
> > > */
> > > -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
> > > +int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests)
> > > {
> > > const struct drm_color_lut *entry;
> > > int i;
> > > @@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
> > >
> > > return 0;
> > > }
> > > -EXPORT_SYMBOL(drm_color_lut_check);
> > > +EXPORT_SYMBOL(drm_color_lut_channels_check);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> > > index dab892d2251ba..a308fe52746ac 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > > @@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
> > > const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> > > const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
> > > int gamma_length, degamma_length;
> > > - u32 gamma_tests, degamma_tests;
> > > + u32 gamma_channels_tests, degamma_channels_tests;
> > >
> > > /* Always allow legacy gamma LUT with no further checking. */
> > > if (crtc_state_is_legacy_gamma(crtc_state))
> > > @@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
> > >
> > > degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> > > gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> > > - degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> > > - gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
> > > + degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
> > > + gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
> > >
> > > - if (check_lut_size(degamma_lut, degamma_length) ||
> > > - check_lut_size(gamma_lut, gamma_length))
> > > - return -EINVAL;
> > > -
> > > - if (drm_color_lut_check(degamma_lut, degamma_tests) ||
> > > - drm_color_lut_check(gamma_lut, gamma_tests))
> > > + if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) ||
> > > + drm_color_lut_channels_check(gamma_lut, gamma_channels_tests))
> > > return -EINVAL;
> > >
> > > return 0;
> > > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> > > index 4045e2507e11c..a22d32a7a8719 100644
> > > --- a/include/drm/drm_atomic_helper.h
> > > +++ b/include/drm/drm_atomic_helper.h
> > > @@ -38,6 +38,7 @@ struct drm_atomic_state;
> > > struct drm_private_obj;
> > > struct drm_private_state;
> > >
> > > +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state);
> > > int drm_atomic_helper_check_modeset(struct drm_device *dev,
> > > struct drm_atomic_state *state);
> > > int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > index 81c298488b0c8..cb1bf361ad3e3 100644
> > > --- a/include/drm/drm_color_mgmt.h
> > > +++ b/include/drm/drm_color_mgmt.h
> > > @@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
> > > enum drm_color_range default_range);
> > >
> > > /**
> > > - * enum drm_color_lut_tests - hw-specific LUT tests to perform
> > > + * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform
> > > *
> > > * The drm_color_lut_check() function takes a bitmask of the values here to
> > > * determine which tests to apply to a userspace-provided LUT.
> > > */
> > > -enum drm_color_lut_tests {
> > > +enum drm_color_lut_channels_tests {
> > > /**
> > > * @DRM_COLOR_LUT_EQUAL_CHANNELS:
> > > *
> > > @@ -119,5 +119,6 @@ enum drm_color_lut_tests {
> > > DRM_COLOR_LUT_NON_DECREASING = BIT(1),
> > > };
> > >
> > > -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests);
> > > +int drm_color_lut_channels_check(const struct drm_property_blob *lut,
> > > + u32 tests);
> > > #endif
> > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > index 2deb15d7e1610..cabd3ef1a6e32 100644
> > > --- a/include/drm/drm_crtc.h
> > > +++ b/include/drm/drm_crtc.h
> > > @@ -1072,6 +1072,17 @@ struct drm_crtc {
> > > /** @funcs: CRTC control functions */
> > > const struct drm_crtc_funcs *funcs;
> > >
> > > + /**
> > > + * @degamma_lut_size: Size of degamma LUT.
> > > + */
> > > + uint32_t degamma_lut_size;
> > > +
> > > + /**
> > > + * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as
> > > + * X, which doesn't support large lut sizes.
> > > + */
> > > + uint32_t gamma_lut_size;
> > > +
> > > /**
> > > * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up
> > > * by calling drm_mode_crtc_set_gamma_size().
> > >
> >
> > Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>
> >
> >
> > Kind regards,
> >
> > Paul
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-10-26 19:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210929194012.3433306-1-markyacoub@chromium.org>
2021-10-13 18:12 ` [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate Mark Yacoub
2021-10-13 18:12 ` [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check Mark Yacoub
2021-10-14 23:42 ` Sean Paul
2021-10-26 1:26 ` [Intel-gfx] [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate Sean Paul
2021-10-26 19:25 ` Mark Yacoub
2021-10-26 12:02 ` Paul Menzel
2021-10-26 19:24 ` Mark Yacoub
2021-10-26 19:26 ` Mark Yacoub
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).