Re: [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check
On Wed, Oct 13, 2021 at 2:12 PM Mark Yacoub wrote:
>
> From: Mark Yacoub
>
> [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
> Signed-off-by: Mark Yacoub
> ---
> .../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, );
> - 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, );
> - 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(>base);
> - if (r)
> - return r;
> -
> degamma_lut = __extract_blob_lut(crtc->base.degamma_lut,
> _size);
> regamma_lut = __extract_blob_lut(crtc->base.gamma_lut, _size);
>
> --
> 2.33.0.882.g93a45727a2-goog
>
[PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check
From: Mark Yacoub
[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
---
.../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, );
- 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, );
- 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(>base);
- if (r)
- return r;
-
degamma_lut = __extract_blob_lut(crtc->base.degamma_lut, _size);
regamma_lut = __extract_blob_lut(crtc->base.gamma_lut, _size);
--
2.33.0.882.g93a45727a2-goog
Re: [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check
On 2021-10-01 15:56, Sean Paul wrote:
> On Wed, Sep 29, 2021 at 03:39:26PM -0400, Mark Yacoub wrote:
>> From: Mark Yacoub
>>
>> [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.
>>
>> Signed-off-by: Mark Yacoub
>> ---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8
>> 1 file changed, 4 insertions(+), 4 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 07adac1a8c42b..96a1d006b777e 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -10683,6 +10683,10 @@ static int amdgpu_dm_atomic_check(struct drm_device
>> *dev,
>> }
>> }
>> #endif
>> +ret = drm_atomic_helper_check_crtc(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);
>>
>> @@ -10692,10 +10696,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);
>
> From a quick glance, I think you can now delete this function. It's called
> from
> amdgpu_dm_update_crtc_color_mgmt() which is part of the commit, so the lut
> sizes
> should have already been checked.
>
I agree. Would be nice if we could drop the extra call in the CM function
(after confirming that it isn't needed).
Harry
> If the call from amdgpu_dm_update_crtc_color_mgmt() is not possible to remove,
> you could replace it with a call to the new helper function. And if _that_ is
> not possible, please make amdgpu_dm_verify_lut_sizes() static :-)
>
> Sean
>
>> -if (ret)
>> -goto fail;
>> -
>> if (!new_crtc_state->enable)
>> continue;
>>
>> --
>> 2.33.0.685.g46640cef36-goog
>>
>
Re: [PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check
On Wed, Sep 29, 2021 at 03:39:26PM -0400, Mark Yacoub wrote:
> From: Mark Yacoub
>
> [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.
>
> Signed-off-by: Mark Yacoub
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8
> 1 file changed, 4 insertions(+), 4 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 07adac1a8c42b..96a1d006b777e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -10683,6 +10683,10 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
> }
> }
> #endif
> + ret = drm_atomic_helper_check_crtc(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);
>
> @@ -10692,10 +10696,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);
>From a quick glance, I think you can now delete this function. It's called from
amdgpu_dm_update_crtc_color_mgmt() which is part of the commit, so the lut sizes
should have already been checked.
If the call from amdgpu_dm_update_crtc_color_mgmt() is not possible to remove,
you could replace it with a call to the new helper function. And if _that_ is
not possible, please make amdgpu_dm_verify_lut_sizes() static :-)
Sean
> - if (ret)
> - goto fail;
> -
> if (!new_crtc_state->enable)
> continue;
>
> --
> 2.33.0.685.g46640cef36-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
[PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check
From: Mark Yacoub
[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.
Signed-off-by: Mark Yacoub
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8
1 file changed, 4 insertions(+), 4 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 07adac1a8c42b..96a1d006b777e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10683,6 +10683,10 @@ static int amdgpu_dm_atomic_check(struct drm_device
*dev,
}
}
#endif
+ ret = drm_atomic_helper_check_crtc(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);
@@ -10692,10 +10696,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;
--
2.33.0.685.g46640cef36-goog
