* [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