* [PATCH] drm/i915/color: Use correct variable to load lut
@ 2024-10-09 6:37 Suraj Kandpal
2024-10-09 6:55 ` Ville Syrjälä
2024-10-09 7:28 ` ✗ Fi.CI.BAT: failure for " Patchwork
0 siblings, 2 replies; 6+ messages in thread
From: Suraj Kandpal @ 2024-10-09 6:37 UTC (permalink / raw)
To: intel-xe, intel-gfx; +Cc: uma.shankar, chaitanya.kumar.borah, Suraj Kandpal
Use the blob variable instead of post_csc_lut as it may end up
being null.
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
drivers/gpu/drm/i915/display/intel_color.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index caf1af039960..22b7090c4f6f 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1503,7 +1503,7 @@ static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
ivb_load_lut_10(crtc_state, pre_csc_lut, PAL_PREC_SPLIT_MODE |
PAL_PREC_INDEX_VALUE(0));
ivb_load_lut_ext_max(crtc_state);
- ivb_load_lut_10(crtc_state, post_csc_lut, PAL_PREC_SPLIT_MODE |
+ ivb_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
PAL_PREC_INDEX_VALUE(512));
break;
case GAMMA_MODE_MODE_10BIT:
@@ -1531,7 +1531,7 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
bdw_load_lut_10(crtc_state, pre_csc_lut, PAL_PREC_SPLIT_MODE |
PAL_PREC_INDEX_VALUE(0));
ivb_load_lut_ext_max(crtc_state);
- bdw_load_lut_10(crtc_state, post_csc_lut, PAL_PREC_SPLIT_MODE |
+ bdw_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
PAL_PREC_INDEX_VALUE(512));
break;
case GAMMA_MODE_MODE_10BIT:
--
2.43.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/color: Use correct variable to load lut
2024-10-09 6:37 [PATCH] drm/i915/color: Use correct variable to load lut Suraj Kandpal
@ 2024-10-09 6:55 ` Ville Syrjälä
2024-10-09 8:46 ` Kandpal, Suraj
2024-10-09 7:28 ` ✗ Fi.CI.BAT: failure for " Patchwork
1 sibling, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2024-10-09 6:55 UTC (permalink / raw)
To: Suraj Kandpal; +Cc: intel-xe, intel-gfx, uma.shankar, chaitanya.kumar.borah
On Wed, Oct 09, 2024 at 12:07:53PM +0530, Suraj Kandpal wrote:
> Use the blob variable instead of post_csc_lut as it may end up
> being null.
Not possible. We always have a LUT for each half in split
gamma mode, and if we don't then someone screwed up much
earlier before we end up here.
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index caf1af039960..22b7090c4f6f 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1503,7 +1503,7 @@ static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
> ivb_load_lut_10(crtc_state, pre_csc_lut, PAL_PREC_SPLIT_MODE |
> PAL_PREC_INDEX_VALUE(0));
> ivb_load_lut_ext_max(crtc_state);
> - ivb_load_lut_10(crtc_state, post_csc_lut, PAL_PREC_SPLIT_MODE |
> + ivb_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> PAL_PREC_INDEX_VALUE(512));
> break;
> case GAMMA_MODE_MODE_10BIT:
> @@ -1531,7 +1531,7 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
> bdw_load_lut_10(crtc_state, pre_csc_lut, PAL_PREC_SPLIT_MODE |
> PAL_PREC_INDEX_VALUE(0));
> ivb_load_lut_ext_max(crtc_state);
> - bdw_load_lut_10(crtc_state, post_csc_lut, PAL_PREC_SPLIT_MODE |
> + bdw_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> PAL_PREC_INDEX_VALUE(512));
> break;
> case GAMMA_MODE_MODE_10BIT:
> --
> 2.43.2
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/color: Use correct variable to load lut
2024-10-09 6:37 [PATCH] drm/i915/color: Use correct variable to load lut Suraj Kandpal
2024-10-09 6:55 ` Ville Syrjälä
@ 2024-10-09 7:28 ` Patchwork
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2024-10-09 7:28 UTC (permalink / raw)
To: Suraj Kandpal; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 7887 bytes --]
== Series Details ==
Series: drm/i915/color: Use correct variable to load lut
URL : https://patchwork.freedesktop.org/series/139749/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_15496 -> Patchwork_139749v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_139749v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_139749v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/index.html
Participating hosts (42 -> 42)
------------------------------
Additional (1): bat-rpls-4
Missing (1): fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_139749v1:
### IGT changes ###
#### Possible regressions ####
* igt@i915_pm_rpm@module-reload:
- bat-rpls-4: NOTRUN -> [DMESG-WARN][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@i915_pm_rpm@module-reload.html
Known issues
------------
Here are the changes found in Patchwork_139749v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@debugfs_test@basic-hwmon:
- bat-rpls-4: NOTRUN -> [SKIP][2] ([i915#9318])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@debugfs_test@basic-hwmon.html
* igt@gem_lmem_swapping@parallel-random-engines:
- bat-rpls-4: NOTRUN -> [SKIP][3] ([i915#4613]) +3 other tests skip
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@gem_lmem_swapping@parallel-random-engines.html
* igt@gem_tiled_pread_basic:
- bat-rpls-4: NOTRUN -> [SKIP][4] ([i915#3282])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@gem_tiled_pread_basic.html
* igt@i915_selftest@live:
- bat-dg2-8: [PASS][5] -> [ABORT][6] ([i915#12133])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-dg2-8/igt@i915_selftest@live.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-dg2-8/igt@i915_selftest@live.html
* igt@i915_selftest@live@active:
- bat-dg2-8: [PASS][7] -> [ABORT][8] ([i915#12305])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-dg2-8/igt@i915_selftest@live@active.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-dg2-8/igt@i915_selftest@live@active.html
* igt@i915_selftest@live@late_gt_pm:
- fi-cfl-8109u: [PASS][9] -> [DMESG-WARN][10] ([i915#11621]) +132 other tests dmesg-warn
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-rpls-4: NOTRUN -> [SKIP][11] ([i915#4103]) +1 other test skip
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
* igt@kms_dsc@dsc-basic:
- bat-rpls-4: NOTRUN -> [SKIP][12] ([i915#3555] / [i915#3840] / [i915#9886])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@kms_dsc@dsc-basic.html
* igt@kms_force_connector_basic@force-load-detect:
- bat-rpls-4: NOTRUN -> [SKIP][13]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_pm_backlight@basic-brightness:
- bat-rpls-4: NOTRUN -> [SKIP][14] ([i915#5354])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@kms_pm_backlight@basic-brightness.html
* igt@kms_psr@psr-primary-page-flip:
- bat-rpls-4: NOTRUN -> [SKIP][15] ([i915#1072] / [i915#9732]) +3 other tests skip
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@kms_psr@psr-primary-page-flip.html
* igt@kms_setmode@basic-clone-single-crtc:
- bat-rpls-4: NOTRUN -> [SKIP][16] ([i915#3555])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-read:
- bat-rpls-4: NOTRUN -> [SKIP][17] ([i915#3708]) +2 other tests skip
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-rpls-4/igt@prime_vgem@basic-read.html
#### Possible fixes ####
* igt@i915_selftest@live:
- {bat-arlh-3}: [ABORT][18] ([i915#12133]) -> [PASS][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-arlh-3/igt@i915_selftest@live.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-arlh-3/igt@i915_selftest@live.html
- bat-twl-1: [INCOMPLETE][20] ([i915#12133]) -> [PASS][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-twl-1/igt@i915_selftest@live.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-twl-1/igt@i915_selftest@live.html
* igt@i915_selftest@live@gt_lrc:
- bat-twl-1: [INCOMPLETE][22] ([i915#10886]) -> [PASS][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-twl-1/igt@i915_selftest@live@gt_lrc.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-twl-1/igt@i915_selftest@live@gt_lrc.html
* igt@i915_selftest@live@workarounds:
- {bat-arlh-3}: [ABORT][24] ([i915#12061]) -> [PASS][25]
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-arlh-3/igt@i915_selftest@live@workarounds.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/bat-arlh-3/igt@i915_selftest@live@workarounds.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1072]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1072
[i915#10886]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10886
[i915#11621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11621
[i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
[i915#12133]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12133
[i915#12305]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12305
[i915#3282]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3282
[i915#3555]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3555
[i915#3708]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3708
[i915#3840]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3840
[i915#4103]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4103
[i915#4613]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4613
[i915#5354]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5354
[i915#9318]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9318
[i915#9732]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9732
[i915#9886]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9886
Build changes
-------------
* Linux: CI_DRM_15496 -> Patchwork_139749v1
CI-20190529: 20190529
CI_DRM_15496: 2c8a08f54befdd831076e0854fe311569c209835 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_8061: 8061
Patchwork_139749v1: 2c8a08f54befdd831076e0854fe311569c209835 @ git://anongit.freedesktop.org/gfx-ci/linux
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139749v1/index.html
[-- Attachment #2: Type: text/html, Size: 9161 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/i915/color: Use correct variable to load lut
2024-10-09 6:55 ` Ville Syrjälä
@ 2024-10-09 8:46 ` Kandpal, Suraj
2024-10-09 8:50 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Kandpal, Suraj @ 2024-10-09 8:46 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
Shankar, Uma, Borah, Chaitanya Kumar
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Wednesday, October 9, 2024 12:25 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Shankar,
> Uma <uma.shankar@intel.com>; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com>
> Subject: Re: [PATCH] drm/i915/color: Use correct variable to load lut
>
> On Wed, Oct 09, 2024 at 12:07:53PM +0530, Suraj Kandpal wrote:
> > Use the blob variable instead of post_csc_lut as it may end up being
> > null.
>
> Not possible. We always have a LUT for each half in split gamma mode, and
> if we don't then someone screwed up much earlier before we end up here.
>
If that is the case then the code line
const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
wouldn't be needed but it has been added presumably because there are times
that we reach here with post csc as null and in that case we at least make sure we are
loading the correct lut hence the fix.
Regards,
Suraj Kandpal
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_color.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > b/drivers/gpu/drm/i915/display/intel_color.c
> > index caf1af039960..22b7090c4f6f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -1503,7 +1503,7 @@ static void ivb_load_luts(const struct
> intel_crtc_state *crtc_state)
> > ivb_load_lut_10(crtc_state, pre_csc_lut,
> PAL_PREC_SPLIT_MODE |
> > PAL_PREC_INDEX_VALUE(0));
> > ivb_load_lut_ext_max(crtc_state);
> > - ivb_load_lut_10(crtc_state, post_csc_lut,
> PAL_PREC_SPLIT_MODE |
> > + ivb_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> > PAL_PREC_INDEX_VALUE(512));
> > break;
> > case GAMMA_MODE_MODE_10BIT:
> > @@ -1531,7 +1531,7 @@ static void bdw_load_luts(const struct
> intel_crtc_state *crtc_state)
> > bdw_load_lut_10(crtc_state, pre_csc_lut,
> PAL_PREC_SPLIT_MODE |
> > PAL_PREC_INDEX_VALUE(0));
> > ivb_load_lut_ext_max(crtc_state);
> > - bdw_load_lut_10(crtc_state, post_csc_lut,
> PAL_PREC_SPLIT_MODE |
> > + bdw_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> > PAL_PREC_INDEX_VALUE(512));
> > break;
> > case GAMMA_MODE_MODE_10BIT:
> > --
> > 2.43.2
>
> --
> Ville Syrjälä
> Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/color: Use correct variable to load lut
2024-10-09 8:46 ` Kandpal, Suraj
@ 2024-10-09 8:50 ` Ville Syrjälä
2024-10-10 4:05 ` Kandpal, Suraj
0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2024-10-09 8:50 UTC (permalink / raw)
To: Kandpal, Suraj
Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
Shankar, Uma, Borah, Chaitanya Kumar
On Wed, Oct 09, 2024 at 08:46:20AM +0000, Kandpal, Suraj wrote:
>
>
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Wednesday, October 9, 2024 12:25 PM
> > To: Kandpal, Suraj <suraj.kandpal@intel.com>
> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Shankar,
> > Uma <uma.shankar@intel.com>; Borah, Chaitanya Kumar
> > <chaitanya.kumar.borah@intel.com>
> > Subject: Re: [PATCH] drm/i915/color: Use correct variable to load lut
> >
> > On Wed, Oct 09, 2024 at 12:07:53PM +0530, Suraj Kandpal wrote:
> > > Use the blob variable instead of post_csc_lut as it may end up being
> > > null.
> >
> > Not possible. We always have a LUT for each half in split gamma mode, and
> > if we don't then someone screwed up much earlier before we end up here.
> >
>
> If that is the case then the code line
> const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
> wouldn't be needed but it has been added presumably because there are times
That is for the non-split gamma cases.
> that we reach here with post csc as null and in that case we at least make sure we are
> loading the correct lut hence the fix.
post_csc_lut is the correct one to load here. Trying to load
the pre_csc_lut twice is just complete nonsense. And if
post_csc_lut could somehow be NULL so could pre_csc_lut.
>
> Regards,
> Suraj Kandpal
>
> > >
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_color.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > > b/drivers/gpu/drm/i915/display/intel_color.c
> > > index caf1af039960..22b7090c4f6f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > > @@ -1503,7 +1503,7 @@ static void ivb_load_luts(const struct
> > intel_crtc_state *crtc_state)
> > > ivb_load_lut_10(crtc_state, pre_csc_lut,
> > PAL_PREC_SPLIT_MODE |
> > > PAL_PREC_INDEX_VALUE(0));
> > > ivb_load_lut_ext_max(crtc_state);
> > > - ivb_load_lut_10(crtc_state, post_csc_lut,
> > PAL_PREC_SPLIT_MODE |
> > > + ivb_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> > > PAL_PREC_INDEX_VALUE(512));
> > > break;
> > > case GAMMA_MODE_MODE_10BIT:
> > > @@ -1531,7 +1531,7 @@ static void bdw_load_luts(const struct
> > intel_crtc_state *crtc_state)
> > > bdw_load_lut_10(crtc_state, pre_csc_lut,
> > PAL_PREC_SPLIT_MODE |
> > > PAL_PREC_INDEX_VALUE(0));
> > > ivb_load_lut_ext_max(crtc_state);
> > > - bdw_load_lut_10(crtc_state, post_csc_lut,
> > PAL_PREC_SPLIT_MODE |
> > > + bdw_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> > > PAL_PREC_INDEX_VALUE(512));
> > > break;
> > > case GAMMA_MODE_MODE_10BIT:
> > > --
> > > 2.43.2
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/i915/color: Use correct variable to load lut
2024-10-09 8:50 ` Ville Syrjälä
@ 2024-10-10 4:05 ` Kandpal, Suraj
0 siblings, 0 replies; 6+ messages in thread
From: Kandpal, Suraj @ 2024-10-10 4:05 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
Shankar, Uma, Borah, Chaitanya Kumar
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Wednesday, October 9, 2024 2:21 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Shankar,
> Uma <uma.shankar@intel.com>; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com>
> Subject: Re: [PATCH] drm/i915/color: Use correct variable to load lut
>
> On Wed, Oct 09, 2024 at 08:46:20AM +0000, Kandpal, Suraj wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Sent: Wednesday, October 9, 2024 12:25 PM
> > > To: Kandpal, Suraj <suraj.kandpal@intel.com>
> > > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org;
> > > Shankar, Uma <uma.shankar@intel.com>; Borah, Chaitanya Kumar
> > > <chaitanya.kumar.borah@intel.com>
> > > Subject: Re: [PATCH] drm/i915/color: Use correct variable to load
> > > lut
> > >
> > > On Wed, Oct 09, 2024 at 12:07:53PM +0530, Suraj Kandpal wrote:
> > > > Use the blob variable instead of post_csc_lut as it may end up
> > > > being null.
> > >
> > > Not possible. We always have a LUT for each half in split gamma
> > > mode, and if we don't then someone screwed up much earlier before
> we end up here.
> > >
> >
> > If that is the case then the code line const struct drm_property_blob
> > *blob = post_csc_lut ?: pre_csc_lut; wouldn't be needed but it has
> > been added presumably because there are times
>
> That is for the non-split gamma cases.
>
> > that we reach here with post csc as null and in that case we at least
> > make sure we are loading the correct lut hence the fix.
>
> post_csc_lut is the correct one to load here. Trying to load the pre_csc_lut
> twice is just complete nonsense. And if post_csc_lut could somehow be
> NULL so could pre_csc_lut.
>
Hi Ville,
do you think it make sense to have a documentation above this code to explain why this made sense
here and not the other places(this being the split gamma case)? to avoid confusion especially from newer
people as myself who aren't as well versed with the history of this function.
Regards,
Suraj Kandpal
> >
> > Regards,
> > Suraj Kandpal
> >
> > > >
> > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_color.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > > > b/drivers/gpu/drm/i915/display/intel_color.c
> > > > index caf1af039960..22b7090c4f6f 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > > > @@ -1503,7 +1503,7 @@ static void ivb_load_luts(const struct
> > > intel_crtc_state *crtc_state)
> > > > ivb_load_lut_10(crtc_state, pre_csc_lut,
> > > PAL_PREC_SPLIT_MODE |
> > > > PAL_PREC_INDEX_VALUE(0));
> > > > ivb_load_lut_ext_max(crtc_state);
> > > > - ivb_load_lut_10(crtc_state, post_csc_lut,
> > > PAL_PREC_SPLIT_MODE |
> > > > + ivb_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> > > > PAL_PREC_INDEX_VALUE(512));
> > > > break;
> > > > case GAMMA_MODE_MODE_10BIT:
> > > > @@ -1531,7 +1531,7 @@ static void bdw_load_luts(const struct
> > > intel_crtc_state *crtc_state)
> > > > bdw_load_lut_10(crtc_state, pre_csc_lut,
> > > PAL_PREC_SPLIT_MODE |
> > > > PAL_PREC_INDEX_VALUE(0));
> > > > ivb_load_lut_ext_max(crtc_state);
> > > > - bdw_load_lut_10(crtc_state, post_csc_lut,
> > > PAL_PREC_SPLIT_MODE |
> > > > + bdw_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> > > > PAL_PREC_INDEX_VALUE(512));
> > > > break;
> > > > case GAMMA_MODE_MODE_10BIT:
> > > > --
> > > > 2.43.2
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
>
> --
> Ville Syrjälä
> Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-10-10 4:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 6:37 [PATCH] drm/i915/color: Use correct variable to load lut Suraj Kandpal
2024-10-09 6:55 ` Ville Syrjälä
2024-10-09 8:46 ` Kandpal, Suraj
2024-10-09 8:50 ` Ville Syrjälä
2024-10-10 4:05 ` Kandpal, Suraj
2024-10-09 7:28 ` ✗ Fi.CI.BAT: failure for " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox