Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
new patch: https://patchwork.freedesktop.org/series/96314/
On Tue, Oct 26, 2021 at 3:24 PM Mark Yacoub wrote:
>
> On Tue, Oct 26, 2021 at 8:02 AM Paul Menzel wrote:
> >
> > Dear Mark,
> >
> >
> > Thank you for your patch.
> >
> > On 13.10.21 20:12, Mark Yacoub wrote:
> > > From: Mark Yacoub
> > >
> > > [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
> > > ---
> > > 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 &&
> > > +
Re: [Intel-gfx] [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
On Mon, Oct 25, 2021 at 9:26 PM Sean Paul wrote:
>
> On Wed, Oct 13, 2021 at 02:12:20PM -0400, Mark Yacoub wrote:
> > From: Mark Yacoub
> >
> > [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
> > ---
> > 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
> > @@
Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
On Tue, Oct 26, 2021 at 8:02 AM Paul Menzel wrote:
>
> Dear Mark,
>
>
> Thank you for your patch.
>
> On 13.10.21 20:12, Mark Yacoub wrote:
> > From: Mark Yacoub
> >
> > [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
> > ---
> > 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
Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
Dear Mark,
Thank you for your patch.
On 13.10.21 20:12, Mark Yacoub wrote:
From: Mark Yacoub
[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
---
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 =
Re: [Intel-gfx] [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
On Wed, Oct 13, 2021 at 02:12:20PM -0400, Mark Yacoub wrote:
> From: Mark Yacoub
>
> [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
> ---
> 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)
>
[PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
From: Mark Yacoub
[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
---
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
Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
On Fri, Oct 1, 2021 at 4:34 PM Sean Paul wrote:
>
> On Wed, Sep 29, 2021 at 03:39:25PM -0400, Mark Yacoub wrote:
> > From: Mark Yacoub
> >
> > [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.
> >
>
> Did you consider extending drm_color_lut_check() with the size checks?
renamed it to be specific to channels. It's HW specific so i thought
of keeping it a separate check if the driver chooses to check it.
Removed the LUT size check that intel uses though.
>
> > Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
> > Tested on Zork(amdgpu) and Jacuzzi(mediatek)
> >
> > Signed-off-by: Mark Yacoub
>
> nit: missing a space between name and email
>
>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 56 +
> > drivers/gpu/drm/drm_color_mgmt.c| 2 ++
> > include/drm/drm_atomic_helper.h | 1 +
> > include/drm/drm_crtc.h | 11 ++
> > 4 files changed, 70 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 2c0c6ec928200..265b9747250d1 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -930,6 +930,58 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_check_planes);
> >
> > +/**
> > + * drm_atomic_helper_check_planes - validate state object for CRTC changes
>
> Ctrl+c/Ctrl+v error here
>
> > + * @state: the driver state object
> > + *
> > + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the
> > new
>
> Are there missing words between "object" and "such"?
>
not really. I was thinking of how to reword it without being too
verbose and nothing sounded good.
I mean I'm checking the object, such as the LUT which is part of this object.
> > + * state holds them.
> > + *
> > + * RETURNS:
> > + * Zero for success or -errno
> > + */
> > +int drm_atomic_helper_check_crtc(struct drm_atomic_state *state)
>
> drm_atomic_helper_check_crtcs to be consistent with
> drm_atomic_helper_check_planes
>
> > +{
> > + 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) {
>
> no space before (
>
> > + if (new_crtc_state->gamma_lut) {
>
> Perhaps gate these with a check of state->color_mgmt_changed first?
done . did it for each check so you can easily expand in the future
and squeeze in more things around those checks as it loops the CRTC
states.
>
> > + 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);
>
> nit: new_state_lut_size and supported_lut_size can be pulled out to top level
> scope
> to avoid re-instantiation on each iteration
>
CRTC is an iterator, so it changes within the loop.
> > +
> > + if (new_state_lut_size != supported_lut_size &&
> > + new_state_lut_size != supported_legacy_lut_size) {
>
> According to the docbook, "If drivers support multiple LUT sizes then they
> should publish the largest size, and sub-sample smaller sized LUTs". So
> should this check be > instead of != ?
>
so IGT tests see it differently, they check for a very specific size,
rather than a range. so if the legacy size is 256 and regular is 1024,
1000 isn't a valid size.
> > + DRM_DEBUG_DRIVER(
>
> drm_dbg_state() is probably more appropriate
>
> > + "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->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 !=
Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
On Fri, Oct 01, 2021 at 04:34:34PM -0400, Sean Paul wrote:
> On Wed, Sep 29, 2021 at 03:39:25PM -0400, Mark Yacoub wrote:
> > From: Mark Yacoub
> >
> > [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.
> >
>
> Did you consider extending drm_color_lut_check() with the size checks?
>
> > Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
> > Tested on Zork(amdgpu) and Jacuzzi(mediatek)
> >
> > Signed-off-by: Mark Yacoub
>
> nit: missing a space between name and email
>
>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 56 +
> > drivers/gpu/drm/drm_color_mgmt.c| 2 ++
> > include/drm/drm_atomic_helper.h | 1 +
> > include/drm/drm_crtc.h | 11 ++
> > 4 files changed, 70 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 2c0c6ec928200..265b9747250d1 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -930,6 +930,58 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_check_planes);
> >
> > +/**
> > + * drm_atomic_helper_check_planes - validate state object for CRTC changes
>
> Ctrl+c/Ctrl+v error here
>
> > + * @state: the driver state object
> > + *
> > + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the
> > new
>
> Are there missing words between "object" and "such"?
>
> > + * state holds them.
> > + *
> > + * RETURNS:
> > + * Zero for success or -errno
> > + */
> > +int drm_atomic_helper_check_crtc(struct drm_atomic_state *state)
>
> drm_atomic_helper_check_crtcs to be consistent with
> drm_atomic_helper_check_planes
>
> > +{
> > + 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) {
>
> no space before (
Ignore this, parsing error on my behalf.
>
> > + if (new_crtc_state->gamma_lut) {
>
> Perhaps gate these with a check of state->color_mgmt_changed first?
>
> > + 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);
>
> nit: new_state_lut_size and supported_lut_size can be pulled out to top level
> scope
> to avoid re-instantiation on each iteration
>
> > +
> > + if (new_state_lut_size != supported_lut_size &&
> > + new_state_lut_size != supported_legacy_lut_size) {
>
> According to the docbook, "If drivers support multiple LUT sizes then they
> should publish the largest size, and sub-sample smaller sized LUTs". So
> should this check be > instead of != ?
>
> > + DRM_DEBUG_DRIVER(
>
> drm_dbg_state() is probably more appropriate
>
> > + "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->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_DEBUG_DRIVER(
>
> drm_dbg_state()
>
> > + "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_crtc);
> > +
> > /**
> > * drm_atomic_helper_check - validate state object
> > * @dev: DRM device
> > @@ -975,6 +1027,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
> > if (ret)
> > return ret;
> >
> > + ret =
Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
On Wed, Sep 29, 2021 at 03:39:25PM -0400, Mark Yacoub wrote:
> From: Mark Yacoub
>
> [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.
>
Did you consider extending drm_color_lut_check() with the size checks?
> Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
> Tested on Zork(amdgpu) and Jacuzzi(mediatek)
>
> Signed-off-by: Mark Yacoub
nit: missing a space between name and email
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 56 +
> drivers/gpu/drm/drm_color_mgmt.c| 2 ++
> include/drm/drm_atomic_helper.h | 1 +
> include/drm/drm_crtc.h | 11 ++
> 4 files changed, 70 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 2c0c6ec928200..265b9747250d1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -930,6 +930,58 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_helper_check_planes);
>
> +/**
> + * drm_atomic_helper_check_planes - validate state object for CRTC changes
Ctrl+c/Ctrl+v error here
> + * @state: the driver state object
> + *
> + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
Are there missing words between "object" and "such"?
> + * state holds them.
> + *
> + * RETURNS:
> + * Zero for success or -errno
> + */
> +int drm_atomic_helper_check_crtc(struct drm_atomic_state *state)
drm_atomic_helper_check_crtcs to be consistent with
drm_atomic_helper_check_planes
> +{
> + 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) {
no space before (
> + if (new_crtc_state->gamma_lut) {
Perhaps gate these with a check of state->color_mgmt_changed first?
> + 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);
nit: new_state_lut_size and supported_lut_size can be pulled out to top level
scope
to avoid re-instantiation on each iteration
> +
> + if (new_state_lut_size != supported_lut_size &&
> + new_state_lut_size != supported_legacy_lut_size) {
According to the docbook, "If drivers support multiple LUT sizes then they
should publish the largest size, and sub-sample smaller sized LUTs". So
should this check be > instead of != ?
> + DRM_DEBUG_DRIVER(
drm_dbg_state() is probably more appropriate
> + "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->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_DEBUG_DRIVER(
drm_dbg_state()
> + "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_crtc);
> +
> /**
> * drm_atomic_helper_check - validate state object
> * @dev: DRM device
> @@ -975,6 +1027,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
> if (ret)
> return ret;
>
> + ret = drm_atomic_helper_check_crtc(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
[PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
From: Mark Yacoub
[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.
Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
Tested on Zork(amdgpu) and Jacuzzi(mediatek)
Signed-off-by: Mark Yacoub
---
drivers/gpu/drm/drm_atomic_helper.c | 56 +
drivers/gpu/drm/drm_color_mgmt.c| 2 ++
include/drm/drm_atomic_helper.h | 1 +
include/drm/drm_crtc.h | 11 ++
4 files changed, 70 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 2c0c6ec928200..265b9747250d1 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -930,6 +930,58 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_atomic_helper_check_planes);
+/**
+ * drm_atomic_helper_check_planes - 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_crtc(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->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_DEBUG_DRIVER(
+ "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->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_DEBUG_DRIVER(
+ "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_crtc);
+
/**
* drm_atomic_helper_check - validate state object
* @dev: DRM device
@@ -975,6 +1027,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
if (ret)
return ret;
+ ret = drm_atomic_helper_check_crtc(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..72a1b628e7cdd 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 = >mode_config;
if (degamma_lut_size) {
+ crtc->degamma_lut_size = degamma_lut_size;
drm_object_attach_property(>base,
config->degamma_lut_property, 0);
drm_object_attach_property(>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(>base,
config->gamma_lut_property, 0);
drm_object_attach_property(>base,
diff --git a/include/drm/drm_atomic_helper.h
