* [PATCH] drm/i915: Preserve value of fec_enable calculated before DSC compute config
@ 2024-08-12 8:24 Chaitanya Kumar Borah
2024-08-12 11:28 ` ✗ Fi.CI.BAT: failure for " Patchwork
2024-08-12 11:53 ` [PATCH] " Jani Nikula
0 siblings, 2 replies; 6+ messages in thread
From: Chaitanya Kumar Borah @ 2024-08-12 8:24 UTC (permalink / raw)
To: intel-gfx, intel-xe; +Cc: imre.deak, chaitanya.kumar.borah
Before DSC compute config, fec_enable value is set in other functions
(e.g. intel_dp_mst_find_vcpi_slots_for_bpp). We are ignoring the value by
OR'ing its value in DSC compute config. One unintended effect of this is
setting fec_enable in UHBR use-cases which is not needed for Intel
hardware. Therefore, change operator to AND.
While at it, add a comment explaining why we don't enable FEC in eDP v1.5.
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 49a37b996530..3fbf9f33c3e4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2298,7 +2298,12 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
&pipe_config->hw.adjusted_mode;
int ret;
- pipe_config->fec_enable = pipe_config->fec_enable ||
+ /*
+ * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
+ * Since, FEC is a bandwidth overhead, continue to not enable it for
+ * eDP. Until, there is a good reason to do so.
+ */
+ pipe_config->fec_enable = pipe_config->fec_enable &&
(!intel_dp_is_edp(intel_dp) &&
intel_dp_supports_fec(intel_dp, connector, pipe_config));
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Preserve value of fec_enable calculated before DSC compute config
2024-08-12 8:24 [PATCH] drm/i915: Preserve value of fec_enable calculated before DSC compute config Chaitanya Kumar Borah
@ 2024-08-12 11:28 ` Patchwork
2024-08-12 11:53 ` [PATCH] " Jani Nikula
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2024-08-12 11:28 UTC (permalink / raw)
To: Chaitanya Kumar Borah; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 3614 bytes --]
== Series Details ==
Series: drm/i915: Preserve value of fec_enable calculated before DSC compute config
URL : https://patchwork.freedesktop.org/series/137153/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_15216 -> Patchwork_137153v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_137153v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_137153v1, 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_137153v1/index.html
Participating hosts (40 -> 37)
------------------------------
Missing (3): bat-arls-1 fi-snb-2520m fi-kbl-8809g
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_137153v1:
### IGT changes ###
#### Possible regressions ####
* igt@kms_dsc@dsc-basic@pipe-a-dp-1:
- bat-dg2-8: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15216/bat-dg2-8/igt@kms_dsc@dsc-basic@pipe-a-dp-1.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_137153v1/bat-dg2-8/igt@kms_dsc@dsc-basic@pipe-a-dp-1.html
Known issues
------------
Here are the changes found in Patchwork_137153v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@hangcheck:
- bat-arls-2: [PASS][3] -> [DMESG-WARN][4] ([i915#11349] / [i915#11378])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15216/bat-arls-2/igt@i915_selftest@live@hangcheck.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_137153v1/bat-arls-2/igt@i915_selftest@live@hangcheck.html
* igt@kms_pipe_crc_basic@nonblocking-crc:
- bat-arls-5: NOTRUN -> [INCOMPLETE][5] ([i915#11320])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_137153v1/bat-arls-5/igt@kms_pipe_crc_basic@nonblocking-crc.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#10196]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10196
[i915#11320]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11320
[i915#11346]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11346
[i915#11349]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11349
[i915#11378]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11378
[i915#11671]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11671
[i915#11681]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11681
[i915#11723]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11723
[i915#11726]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11726
[i915#8809]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/8809
Build changes
-------------
* Linux: CI_DRM_15216 -> Patchwork_137153v1
CI-20190529: 20190529
CI_DRM_15216: 1daf2018001da75c5522669aebdb68ba9a353ea6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7966: f16c5f500adc5fa41bd52a3ef2a84165da4fb596 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_137153v1: 1daf2018001da75c5522669aebdb68ba9a353ea6 @ git://anongit.freedesktop.org/gfx-ci/linux
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_137153v1/index.html
[-- Attachment #2: Type: text/html, Size: 3738 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Preserve value of fec_enable calculated before DSC compute config
2024-08-12 8:24 [PATCH] drm/i915: Preserve value of fec_enable calculated before DSC compute config Chaitanya Kumar Borah
2024-08-12 11:28 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2024-08-12 11:53 ` Jani Nikula
2024-08-12 13:03 ` Borah, Chaitanya Kumar
1 sibling, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2024-08-12 11:53 UTC (permalink / raw)
To: Chaitanya Kumar Borah, intel-gfx, intel-xe
Cc: imre.deak, chaitanya.kumar.borah
On Mon, 12 Aug 2024, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> wrote:
> Before DSC compute config, fec_enable value is set in other functions
> (e.g. intel_dp_mst_find_vcpi_slots_for_bpp). We are ignoring the value by
> OR'ing its value in DSC compute config. One unintended effect of this is
> setting fec_enable in UHBR use-cases which is not needed for Intel
> hardware. Therefore, change operator to AND.
>
> While at it, add a comment explaining why we don't enable FEC in eDP v1.5.
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 49a37b996530..3fbf9f33c3e4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2298,7 +2298,12 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> &pipe_config->hw.adjusted_mode;
> int ret;
>
> - pipe_config->fec_enable = pipe_config->fec_enable ||
> + /*
> + * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
> + * Since, FEC is a bandwidth overhead, continue to not enable it for
> + * eDP. Until, there is a good reason to do so.
> + */
> + pipe_config->fec_enable = pipe_config->fec_enable &&
> (!intel_dp_is_edp(intel_dp) &&
> intel_dp_supports_fec(intel_dp, connector, pipe_config));
With this change, FEC will only be enabled for non-UHBR MST DP and
nothing else. This is the place where SST DP DSC gets FEC enabled.
BR,
Jani.
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/i915: Preserve value of fec_enable calculated before DSC compute config
2024-08-12 11:53 ` [PATCH] " Jani Nikula
@ 2024-08-12 13:03 ` Borah, Chaitanya Kumar
2024-08-14 8:45 ` Jani Nikula
0 siblings, 1 reply; 6+ messages in thread
From: Borah, Chaitanya Kumar @ 2024-08-12 13:03 UTC (permalink / raw)
To: Jani Nikula, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org
Cc: Deak, Imre
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Monday, August 12, 2024 5:24 PM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel-
> gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> Cc: Deak, Imre <imre.deak@intel.com>; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com>
> Subject: Re: [PATCH] drm/i915: Preserve value of fec_enable calculated before
> DSC compute config
>
> On Mon, 12 Aug 2024, Chaitanya Kumar Borah
> <chaitanya.kumar.borah@intel.com> wrote:
> > Before DSC compute config, fec_enable value is set in other functions
> > (e.g. intel_dp_mst_find_vcpi_slots_for_bpp). We are ignoring the value
> > by OR'ing its value in DSC compute config. One unintended effect of
> > this is setting fec_enable in UHBR use-cases which is not needed for
> > Intel hardware. Therefore, change operator to AND.
> >
> > While at it, add a comment explaining why we don't enable FEC in eDP v1.5.
> >
> > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 49a37b996530..3fbf9f33c3e4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -2298,7 +2298,12 @@ int intel_dp_dsc_compute_config(struct
> intel_dp *intel_dp,
> > &pipe_config->hw.adjusted_mode;
> > int ret;
> >
> > - pipe_config->fec_enable = pipe_config->fec_enable ||
> > + /*
> > + * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
> > + * Since, FEC is a bandwidth overhead, continue to not enable it for
> > + * eDP. Until, there is a good reason to do so.
> > + */
> > + pipe_config->fec_enable = pipe_config->fec_enable &&
> > (!intel_dp_is_edp(intel_dp) &&
> > intel_dp_supports_fec(intel_dp, connector, pipe_config));
>
> With this change, FEC will only be enabled for non-UHBR MST DP and nothing
> else. This is the place where SST DP DSC gets FEC enabled.
Thank you Jani for pointing it out.
Something like this should work?
pipe_config->fec_enable = pipe_config->fec_enable ||
(!intel_dp_is_edp(intel_dp) &&
intel_dp_supports_fec(intel_dp, connector, pipe_config) && !intel_dp_is_uhbr(pipe_config));
Regards
Chaitanya
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/i915: Preserve value of fec_enable calculated before DSC compute config
2024-08-12 13:03 ` Borah, Chaitanya Kumar
@ 2024-08-14 8:45 ` Jani Nikula
2024-08-22 6:20 ` Borah, Chaitanya Kumar
0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2024-08-14 8:45 UTC (permalink / raw)
To: Borah, Chaitanya Kumar, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org
Cc: Deak, Imre
On Mon, 12 Aug 2024, "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com> wrote:
>> -----Original Message-----
>> From: Jani Nikula <jani.nikula@linux.intel.com>
>> Sent: Monday, August 12, 2024 5:24 PM
>> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel-
>> gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
>> Cc: Deak, Imre <imre.deak@intel.com>; Borah, Chaitanya Kumar
>> <chaitanya.kumar.borah@intel.com>
>> Subject: Re: [PATCH] drm/i915: Preserve value of fec_enable calculated before
>> DSC compute config
>>
>> On Mon, 12 Aug 2024, Chaitanya Kumar Borah
>> <chaitanya.kumar.borah@intel.com> wrote:
>> > Before DSC compute config, fec_enable value is set in other functions
>> > (e.g. intel_dp_mst_find_vcpi_slots_for_bpp). We are ignoring the value
>> > by OR'ing its value in DSC compute config. One unintended effect of
>> > this is setting fec_enable in UHBR use-cases which is not needed for
>> > Intel hardware. Therefore, change operator to AND.
>> >
>> > While at it, add a comment explaining why we don't enable FEC in eDP v1.5.
>> >
>> > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++-
>> > 1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> > b/drivers/gpu/drm/i915/display/intel_dp.c
>> > index 49a37b996530..3fbf9f33c3e4 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > @@ -2298,7 +2298,12 @@ int intel_dp_dsc_compute_config(struct
>> intel_dp *intel_dp,
>> > &pipe_config->hw.adjusted_mode;
>> > int ret;
>> >
>> > - pipe_config->fec_enable = pipe_config->fec_enable ||
>> > + /*
>> > + * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
>> > + * Since, FEC is a bandwidth overhead, continue to not enable it for
>> > + * eDP. Until, there is a good reason to do so.
>> > + */
>> > + pipe_config->fec_enable = pipe_config->fec_enable &&
>> > (!intel_dp_is_edp(intel_dp) &&
>> > intel_dp_supports_fec(intel_dp, connector, pipe_config));
>>
>> With this change, FEC will only be enabled for non-UHBR MST DP and nothing
>> else. This is the place where SST DP DSC gets FEC enabled.
>
> Thank you Jani for pointing it out.
>
> Something like this should work?
>
> pipe_config->fec_enable = pipe_config->fec_enable ||
> (!intel_dp_is_edp(intel_dp) &&
> intel_dp_supports_fec(intel_dp, connector, pipe_config) && !intel_dp_is_uhbr(pipe_config));
Technically yes, although I dislike the fact that we now have fec_enable
and its rules split to multiple locations.
BR,
Jani.
>
> Regards
>
> Chaitanya
>
>>
>> BR,
>> Jani.
>>
>>
>> --
>> Jani Nikula, Intel
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/i915: Preserve value of fec_enable calculated before DSC compute config
2024-08-14 8:45 ` Jani Nikula
@ 2024-08-22 6:20 ` Borah, Chaitanya Kumar
0 siblings, 0 replies; 6+ messages in thread
From: Borah, Chaitanya Kumar @ 2024-08-22 6:20 UTC (permalink / raw)
To: Jani Nikula, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org
Cc: Deak, Imre
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Wednesday, August 14, 2024 2:15 PM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel-
> gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> Cc: Deak, Imre <imre.deak@intel.com>
> Subject: RE: [PATCH] drm/i915: Preserve value of fec_enable calculated before
> DSC compute config
>
> On Mon, 12 Aug 2024, "Borah, Chaitanya Kumar"
> <chaitanya.kumar.borah@intel.com> wrote:
> >> -----Original Message-----
> >> From: Jani Nikula <jani.nikula@linux.intel.com>
> >> Sent: Monday, August 12, 2024 5:24 PM
> >> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel-
> >> gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> >> Cc: Deak, Imre <imre.deak@intel.com>; Borah, Chaitanya Kumar
> >> <chaitanya.kumar.borah@intel.com>
> >> Subject: Re: [PATCH] drm/i915: Preserve value of fec_enable
> >> calculated before DSC compute config
> >>
> >> On Mon, 12 Aug 2024, Chaitanya Kumar Borah
> >> <chaitanya.kumar.borah@intel.com> wrote:
> >> > Before DSC compute config, fec_enable value is set in other
> >> > functions (e.g. intel_dp_mst_find_vcpi_slots_for_bpp). We are
> >> > ignoring the value by OR'ing its value in DSC compute config. One
> >> > unintended effect of this is setting fec_enable in UHBR use-cases
> >> > which is not needed for Intel hardware. Therefore, change operator to
> AND.
> >> >
> >> > While at it, add a comment explaining why we don't enable FEC in eDP
> v1.5.
> >> >
> >> > Signed-off-by: Chaitanya Kumar Borah
> >> > <chaitanya.kumar.borah@intel.com>
> >> > ---
> >> > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++-
> >> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> > b/drivers/gpu/drm/i915/display/intel_dp.c
> >> > index 49a37b996530..3fbf9f33c3e4 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> > @@ -2298,7 +2298,12 @@ int intel_dp_dsc_compute_config(struct
> >> intel_dp *intel_dp,
> >> > &pipe_config->hw.adjusted_mode;
> >> > int ret;
> >> >
> >> > - pipe_config->fec_enable = pipe_config->fec_enable ||
> >> > + /*
> >> > + * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
> >> > + * Since, FEC is a bandwidth overhead, continue to not enable it for
> >> > + * eDP. Until, there is a good reason to do so.
> >> > + */
> >> > + pipe_config->fec_enable = pipe_config->fec_enable &&
> >> > (!intel_dp_is_edp(intel_dp) &&
> >> > intel_dp_supports_fec(intel_dp, connector, pipe_config));
> >>
> >> With this change, FEC will only be enabled for non-UHBR MST DP and
> >> nothing else. This is the place where SST DP DSC gets FEC enabled.
> >
> > Thank you Jani for pointing it out.
> >
> > Something like this should work?
> >
> > pipe_config->fec_enable = pipe_config->fec_enable ||
> > (!intel_dp_is_edp(intel_dp) &&
> > intel_dp_supports_fec(intel_dp, connector, pipe_config) &&
> > !intel_dp_is_uhbr(pipe_config));
>
> Technically yes, although I dislike the fact that we now have fec_enable and its
> rules split to multiple locations.
>
I agree. if I understand correctly this should be covered by the larger separation of DP-MST and MTP planned for UHBR.
Until that time, hopefully [1] is acceptable.
[1] https://patchwork.freedesktop.org/series/137611/
Regards
Chaitanya
> BR,
> Jani.
>
>
> >
> > Regards
> >
> > Chaitanya
> >
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >> --
> >> Jani Nikula, Intel
>
> --
> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-22 6:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-12 8:24 [PATCH] drm/i915: Preserve value of fec_enable calculated before DSC compute config Chaitanya Kumar Borah
2024-08-12 11:28 ` ✗ Fi.CI.BAT: failure for " Patchwork
2024-08-12 11:53 ` [PATCH] " Jani Nikula
2024-08-12 13:03 ` Borah, Chaitanya Kumar
2024-08-14 8:45 ` Jani Nikula
2024-08-22 6:20 ` Borah, Chaitanya Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox