* [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
@ 2023-04-11 17:34 Manasi Navare
2023-04-11 17:42 ` Ville Syrjälä
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Manasi Navare @ 2023-04-11 17:34 UTC (permalink / raw)
To: intel-gfx
In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
we return 0 instead of returning an err code of -EINVAL.
This throws off the logic in the calling function. Fix this by returning
-EINVAL in case bpc < 0 which would be an error.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Cc: Sean Paul <sean@poorly.run>
Signed-off-by: Manasi Navare <navaremanasi@chromium.org>
---
drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index f0bace9d98a1..f6546292e7c6 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1280,7 +1280,7 @@ static int intel_dp_max_bpp(struct intel_dp *intel_dp,
max_hdmi_bpc = intel_dp_hdmi_compute_bpc(intel_dp, crtc_state, bpc,
respect_downstream_limits);
if (max_hdmi_bpc < 0)
- return 0;
+ return -EINVAL;
bpc = min(bpc, max_hdmi_bpc);
}
--
2.40.0.577.gac1e443424-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-11 17:34 [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0 Manasi Navare
@ 2023-04-11 17:42 ` Ville Syrjälä
2023-04-12 0:07 ` Manasi Navare
2023-04-12 0:28 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2023-04-12 13:44 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2023-04-11 17:42 UTC (permalink / raw)
To: Manasi Navare; +Cc: intel-gfx
On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
> In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
> we return 0 instead of returning an err code of -EINVAL.
> This throws off the logic in the calling function.
What logic? The caller doesn't expect to get an error.
> Fix this by returning
> -EINVAL in case bpc < 0 which would be an error.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Cc: Sean Paul <sean@poorly.run>
> Signed-off-by: Manasi Navare <navaremanasi@chromium.org>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index f0bace9d98a1..f6546292e7c6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1280,7 +1280,7 @@ static int intel_dp_max_bpp(struct intel_dp *intel_dp,
> max_hdmi_bpc = intel_dp_hdmi_compute_bpc(intel_dp, crtc_state, bpc,
> respect_downstream_limits);
> if (max_hdmi_bpc < 0)
> - return 0;
> + return -EINVAL;
>
> bpc = min(bpc, max_hdmi_bpc);
> }
> --
> 2.40.0.577.gac1e443424-goog
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-11 17:42 ` Ville Syrjälä
@ 2023-04-12 0:07 ` Manasi Navare
2023-04-12 5:22 ` Ville Syrjälä
0 siblings, 1 reply; 14+ messages in thread
From: Manasi Navare @ 2023-04-12 0:07 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
> > In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
> > we return 0 instead of returning an err code of -EINVAL.
> > This throws off the logic in the calling function.
>
> What logic? The caller doesn't expect to get an error.
If this returns a 0, we end up using limits.max_bpp = 0 and in
intel_dp_compute_link_config_wide(),
since max_bpp is 0, it exits this for loop:
for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns
-EINVAL which then wrongly goes to enable DSC even when link BW is
sufficient without DSC.
Manasi
>
> > Fix this by returning
> > -EINVAL in case bpc < 0 which would be an error.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Signed-off-by: Manasi Navare <navaremanasi@chromium.org>
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index f0bace9d98a1..f6546292e7c6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1280,7 +1280,7 @@ static int intel_dp_max_bpp(struct intel_dp *intel_dp,
> > max_hdmi_bpc = intel_dp_hdmi_compute_bpc(intel_dp, crtc_state, bpc,
> > respect_downstream_limits);
> > if (max_hdmi_bpc < 0)
> > - return 0;
> > + return -EINVAL;
> >
> > bpc = min(bpc, max_hdmi_bpc);
> > }
> > --
> > 2.40.0.577.gac1e443424-goog
>
> --
> Ville Syrjälä
> Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: Return correct err code for bpc < 0
2023-04-11 17:34 [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0 Manasi Navare
2023-04-11 17:42 ` Ville Syrjälä
@ 2023-04-12 0:28 ` Patchwork
2023-04-12 13:44 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-04-12 0:28 UTC (permalink / raw)
To: Manasi Navare; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 7543 bytes --]
== Series Details ==
Series: drm/i915/display: Return correct err code for bpc < 0
URL : https://patchwork.freedesktop.org/series/116331/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12993 -> Patchwork_116331v1
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/index.html
Participating hosts (37 -> 36)
------------------------------
Missing (1): fi-snb-2520m
Known issues
------------
Here are the changes found in Patchwork_116331v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_pm_rps@basic-api:
- bat-dg2-11: [PASS][1] -> [FAIL][2] ([i915#8308])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/bat-dg2-11/igt@i915_pm_rps@basic-api.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-dg2-11/igt@i915_pm_rps@basic-api.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-kbl-soraka: [PASS][3] -> [DMESG-FAIL][4] ([i915#5334] / [i915#7872])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@gt_lrc:
- bat-dg2-11: [PASS][5] -> [INCOMPLETE][6] ([i915#7609] / [i915#7913])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/bat-dg2-11/igt@i915_selftest@live@gt_lrc.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-dg2-11/igt@i915_selftest@live@gt_lrc.html
* igt@i915_selftest@live@reset:
- bat-rpls-2: [PASS][7] -> [ABORT][8] ([i915#4983] / [i915#7913] / [i915#7981])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/bat-rpls-2/igt@i915_selftest@live@reset.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-rpls-2/igt@i915_selftest@live@reset.html
* igt@i915_selftest@live@workarounds:
- bat-dg1-7: [PASS][9] -> [ABORT][10] ([i915#4983])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/bat-dg1-7/igt@i915_selftest@live@workarounds.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-dg1-7/igt@i915_selftest@live@workarounds.html
* igt@i915_suspend@basic-s3-without-i915:
- bat-dg2-8: NOTRUN -> [SKIP][11] ([i915#6645])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-dg2-8/igt@i915_suspend@basic-s3-without-i915.html
* igt@kms_chamelium_hpd@common-hpd-after-suspend:
- bat-dg2-8: NOTRUN -> [SKIP][12] ([i915#7828])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-dg2-8/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
- bat-rpls-1: NOTRUN -> [SKIP][13] ([i915#7828])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-rpls-1/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
* igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
- bat-dg2-11: NOTRUN -> [SKIP][14] ([i915#5354])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html
* igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1:
- bat-dg2-8: [PASS][15] -> [FAIL][16] ([i915#7932])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1.html
* igt@kms_pipe_crc_basic@suspend-read-crc:
- bat-rpls-1: NOTRUN -> [SKIP][17] ([i915#1845])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-rpls-1/igt@kms_pipe_crc_basic@suspend-read-crc.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s3@smem:
- bat-rpls-1: [ABORT][18] ([i915#6687] / [i915#7978]) -> [PASS][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [DMESG-FAIL][20] ([i915#5334]) -> [PASS][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@hangcheck:
- bat-dg2-8: [ABORT][22] ([i915#7913] / [i915#7979]) -> [PASS][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/bat-dg2-8/igt@i915_selftest@live@hangcheck.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-dg2-8/igt@i915_selftest@live@hangcheck.html
* igt@i915_selftest@live@slpc:
- bat-rplp-1: [DMESG-FAIL][24] ([i915#6367] / [i915#7913]) -> [PASS][25]
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/bat-rplp-1/igt@i915_selftest@live@slpc.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-rplp-1/igt@i915_selftest@live@slpc.html
#### Warnings ####
* igt@i915_selftest@live@slpc:
- bat-rpls-1: [DMESG-FAIL][26] ([i915#6367]) -> [DMESG-FAIL][27] ([i915#6367] / [i915#7996])
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/bat-rpls-1/igt@i915_selftest@live@slpc.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/bat-rpls-1/igt@i915_selftest@live@slpc.html
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
[i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
[i915#7609]: https://gitlab.freedesktop.org/drm/intel/issues/7609
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
[i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
[i915#7979]: https://gitlab.freedesktop.org/drm/intel/issues/7979
[i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981
[i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996
[i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308
Build changes
-------------
* Linux: CI_DRM_12993 -> Patchwork_116331v1
CI-20190529: 20190529
CI_DRM_12993: 3f6d1a580787c3aa8c9c7f174bdce5b055d6d724 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7250: 2da179d399d83a6859a89176d83b7ec1d71fe27a @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116331v1: 3f6d1a580787c3aa8c9c7f174bdce5b055d6d724 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
01bc7661c095 drm/i915/display: Return correct err code for bpc < 0
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/index.html
[-- Attachment #2: Type: text/html, Size: 9061 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-12 0:07 ` Manasi Navare
@ 2023-04-12 5:22 ` Ville Syrjälä
2023-04-13 15:23 ` Manasi Navare
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2023-04-12 5:22 UTC (permalink / raw)
To: Manasi Navare; +Cc: intel-gfx
On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote:
> On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
> > > In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
> > > we return 0 instead of returning an err code of -EINVAL.
> > > This throws off the logic in the calling function.
> >
> > What logic? The caller doesn't expect to get an error.
>
> If this returns a 0, we end up using limits.max_bpp = 0 and in
> intel_dp_compute_link_config_wide(),
> since max_bpp is 0, it exits this for loop:
>
> for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns
> -EINVAL which then wrongly goes to enable DSC even when link BW is
> sufficient without DSC.
And how woud max_bpp<0 prevent that?
The real problem seems to be that the DSC code totally
ignores bpp limits.
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/display: Return correct err code for bpc < 0
2023-04-11 17:34 [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0 Manasi Navare
2023-04-11 17:42 ` Ville Syrjälä
2023-04-12 0:28 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
@ 2023-04-12 13:44 ` Patchwork
2 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-04-12 13:44 UTC (permalink / raw)
To: Manasi Navare; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 14823 bytes --]
== Series Details ==
Series: drm/i915/display: Return correct err code for bpc < 0
URL : https://patchwork.freedesktop.org/series/116331/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12993_full -> Patchwork_116331v1_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (7 -> 7)
------------------------------
No changes in participating hosts
Known issues
------------
Here are the changes found in Patchwork_116331v1_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_fair@basic-none-solo@rcs0:
- shard-apl: [PASS][1] -> [FAIL][2] ([i915#2842])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-apl7/igt@gem_exec_fair@basic-none-solo@rcs0.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-apl4/igt@gem_exec_fair@basic-none-solo@rcs0.html
* igt@gem_huc_copy@huc-copy:
- shard-glk: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-glk5/igt@gem_huc_copy@huc-copy.html
* igt@gem_userptr_blits@access-control:
- shard-glk: NOTRUN -> [SKIP][4] ([fdo#109271]) +8 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-glk1/igt@gem_userptr_blits@access-control.html
* igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc:
- shard-glk: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#3886]) +1 similar issue
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-glk5/igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc.html
* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
- shard-apl: [PASS][6] -> [FAIL][7] ([i915#2346])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-apl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-apl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
* igt@v3d/v3d_wait_bo@unused-bo-1ns:
- shard-apl: NOTRUN -> [SKIP][8] ([fdo#109271]) +10 similar issues
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-apl1/igt@v3d/v3d_wait_bo@unused-bo-1ns.html
#### Possible fixes ####
* igt@gem_exec_endless@dispatch@bcs0:
- {shard-tglu}: [TIMEOUT][9] ([i915#3778]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-tglu-9/igt@gem_exec_endless@dispatch@bcs0.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-tglu-9/igt@gem_exec_endless@dispatch@bcs0.html
* igt@gem_exec_fair@basic-deadline:
- shard-glk: [FAIL][11] ([i915#2846]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-glk7/igt@gem_exec_fair@basic-deadline.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-glk1/igt@gem_exec_fair@basic-deadline.html
* igt@gem_exec_fair@basic-pace-share@rcs0:
- shard-glk: [FAIL][13] ([i915#2842]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-glk9/igt@gem_exec_fair@basic-pace-share@rcs0.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-glk2/igt@gem_exec_fair@basic-pace-share@rcs0.html
* igt@gem_exec_fair@basic-pace-solo@rcs0:
- shard-apl: [FAIL][15] ([i915#2842]) -> [PASS][16] +1 similar issue
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-apl6/igt@gem_exec_fair@basic-pace-solo@rcs0.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-apl4/igt@gem_exec_fair@basic-pace-solo@rcs0.html
* igt@gen9_exec_parse@allowed-single:
- shard-glk: [ABORT][17] ([i915#5566]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-glk7/igt@gen9_exec_parse@allowed-single.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-glk5/igt@gen9_exec_parse@allowed-single.html
* igt@i915_pm_dc@dc6-dpms:
- {shard-tglu}: [FAIL][19] ([i915#3989] / [i915#454]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-tglu-8/igt@i915_pm_dc@dc6-dpms.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-tglu-9/igt@i915_pm_dc@dc6-dpms.html
* igt@i915_pm_rpm@dpms-non-lpsp:
- {shard-rkl}: [SKIP][21] ([i915#1397]) -> [PASS][22] +1 similar issue
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-rkl-7/igt@i915_pm_rpm@dpms-non-lpsp.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-rkl-2/igt@i915_pm_rpm@dpms-non-lpsp.html
* igt@i915_pm_rpm@modeset-lpsp-stress-no-wait:
- {shard-dg1}: [SKIP][23] ([i915#1397]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-dg1-17/igt@i915_pm_rpm@modeset-lpsp-stress-no-wait.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-dg1-14/igt@i915_pm_rpm@modeset-lpsp-stress-no-wait.html
* igt@i915_pm_rps@engine-order:
- shard-apl: [FAIL][25] ([i915#6537]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-apl4/igt@i915_pm_rps@engine-order.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-apl2/igt@i915_pm_rps@engine-order.html
* igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip:
- {shard-dg1}: [FAIL][27] ([i915#7959]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-dg1-14/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-dg1-16/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
* igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
- shard-apl: [ABORT][29] ([i915#180]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-apl1/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
* igt@kms_plane_scaling@i915-max-src-size@pipe-a-hdmi-a-1:
- {shard-tglu}: [FAIL][31] ([i915#8292]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-tglu-8/igt@kms_plane_scaling@i915-max-src-size@pipe-a-hdmi-a-1.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-tglu-5/igt@kms_plane_scaling@i915-max-src-size@pipe-a-hdmi-a-1.html
* igt@perf@stress-open-close@0-rcs0:
- shard-glk: [ABORT][33] ([i915#5213]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12993/shard-glk7/igt@perf@stress-open-close@0-rcs0.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/shard-glk1/igt@perf@stress-open-close@0-rcs0.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
[fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
[fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
[fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
[fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
[i915#1902]: https://gitlab.freedesktop.org/drm/intel/issues/1902
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2433]: https://gitlab.freedesktop.org/drm/intel/issues/2433
[i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
[i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
[i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
[i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
[i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
[i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
[i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
[i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
[i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
[i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
[i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
[i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
[i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
[i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
[i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3778]: https://gitlab.freedesktop.org/drm/intel/issues/3778
[i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
[i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#3936]: https://gitlab.freedesktop.org/drm/intel/issues/3936
[i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
[i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
[i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
[i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
[i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
[i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
[i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
[i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
[i915#4859]: https://gitlab.freedesktop.org/drm/intel/issues/4859
[i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
[i915#4879]: https://gitlab.freedesktop.org/drm/intel/issues/4879
[i915#4958]: https://gitlab.freedesktop.org/drm/intel/issues/4958
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5213]: https://gitlab.freedesktop.org/drm/intel/issues/5213
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
[i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
[i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
[i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
[i915#5431]: https://gitlab.freedesktop.org/drm/intel/issues/5431
[i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
[i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
[i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
[i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
[i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
[i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
[i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
[i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
[i915#6537]: https://gitlab.freedesktop.org/drm/intel/issues/6537
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6590]: https://gitlab.freedesktop.org/drm/intel/issues/6590
[i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
[i915#7178]: https://gitlab.freedesktop.org/drm/intel/issues/7178
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
[i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7959]: https://gitlab.freedesktop.org/drm/intel/issues/7959
[i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
[i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
[i915#8253]: https://gitlab.freedesktop.org/drm/intel/issues/8253
[i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
[i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308
Build changes
-------------
* Linux: CI_DRM_12993 -> Patchwork_116331v1
CI-20190529: 20190529
CI_DRM_12993: 3f6d1a580787c3aa8c9c7f174bdce5b055d6d724 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7250: 2da179d399d83a6859a89176d83b7ec1d71fe27a @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116331v1: 3f6d1a580787c3aa8c9c7f174bdce5b055d6d724 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116331v1/index.html
[-- Attachment #2: Type: text/html, Size: 10392 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-12 5:22 ` Ville Syrjälä
@ 2023-04-13 15:23 ` Manasi Navare
2023-04-17 22:48 ` Manasi Navare
0 siblings, 1 reply; 14+ messages in thread
From: Manasi Navare @ 2023-04-13 15:23 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Tue, Apr 11, 2023 at 10:22 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote:
> > On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
> > > > In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
> > > > we return 0 instead of returning an err code of -EINVAL.
> > > > This throws off the logic in the calling function.
> > >
> > > What logic? The caller doesn't expect to get an error.
> >
> > If this returns a 0, we end up using limits.max_bpp = 0 and in
> > intel_dp_compute_link_config_wide(),
> > since max_bpp is 0, it exits this for loop:
> >
> > for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns
> > -EINVAL which then wrongly goes to enable DSC even when link BW is
> > sufficient without DSC.
>
> And how woud max_bpp<0 prevent that?
>
> The real problem seems to be that the DSC code totally
> ignores bpp limits.
Hi Ville,
So I see a few concerns/questions:
- Why is the Max bpp value 0 in intel_dp_max_bpp, is that a valid case
and how should our link configurations handle that case when max_bpp
is 0?
- This is happening in a bug I am looking at with HDMI PCON, @Ankit
Nautiyal have we ever seen something similar where max_bpp for HDMi
PCON
is returned 0?
- I dont think its a problem with DSC code, but rather
intel_dp_compute_link_config() outer for loop where we vary
from max_bpp to min_bpp and see if any bpp works with available link
bw, how should we handle this when max_bpp = 0 if you are saying thats
a valid case?
- In this patch if I return -EINVAL instead of 0, then atleast the
entire encoder_config will fail and that will fail the modeset, since
it assumes max_bpp cannot be 0
Could you please help answer above concerns and how to handle max bpp
= 0 case if that is valid? This patch is simply making that invalid
resulting into modeset failure
Manasi
>
> --
> Ville Syrjälä
> Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-13 15:23 ` Manasi Navare
@ 2023-04-17 22:48 ` Manasi Navare
2023-04-18 12:46 ` Ville Syrjälä
0 siblings, 1 reply; 14+ messages in thread
From: Manasi Navare @ 2023-04-17 22:48 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Hi Ville,
Could you suggest how to handle the intel_dp_link_compute_config()
when the max_bpp is returned as 0, currently
it just exits the loop and returns a -EINVAL and this triggers the DSC path.
While we should be completely failing the modeset and encoder_config
in this case instead of trying DSC, correct?
Manasi
On Thu, Apr 13, 2023 at 8:23 AM Manasi Navare <navaremanasi@chromium.org> wrote:
>
> On Tue, Apr 11, 2023 at 10:22 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote:
> > > On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
> > > <ville.syrjala@linux.intel.com> wrote:
> > > >
> > > > On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
> > > > > In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
> > > > > we return 0 instead of returning an err code of -EINVAL.
> > > > > This throws off the logic in the calling function.
> > > >
> > > > What logic? The caller doesn't expect to get an error.
> > >
> > > If this returns a 0, we end up using limits.max_bpp = 0 and in
> > > intel_dp_compute_link_config_wide(),
> > > since max_bpp is 0, it exits this for loop:
> > >
> > > for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns
> > > -EINVAL which then wrongly goes to enable DSC even when link BW is
> > > sufficient without DSC.
> >
> > And how woud max_bpp<0 prevent that?
> >
> > The real problem seems to be that the DSC code totally
> > ignores bpp limits.
>
> Hi Ville,
>
> So I see a few concerns/questions:
> - Why is the Max bpp value 0 in intel_dp_max_bpp, is that a valid case
> and how should our link configurations handle that case when max_bpp
> is 0?
> - This is happening in a bug I am looking at with HDMI PCON, @Ankit
> Nautiyal have we ever seen something similar where max_bpp for HDMi
> PCON
> is returned 0?
> - I dont think its a problem with DSC code, but rather
> intel_dp_compute_link_config() outer for loop where we vary
> from max_bpp to min_bpp and see if any bpp works with available link
> bw, how should we handle this when max_bpp = 0 if you are saying thats
> a valid case?
> - In this patch if I return -EINVAL instead of 0, then atleast the
> entire encoder_config will fail and that will fail the modeset, since
> it assumes max_bpp cannot be 0
>
> Could you please help answer above concerns and how to handle max bpp
> = 0 case if that is valid? This patch is simply making that invalid
> resulting into modeset failure
>
> Manasi
> >
> > --
> > Ville Syrjälä
> > Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-17 22:48 ` Manasi Navare
@ 2023-04-18 12:46 ` Ville Syrjälä
2023-04-18 14:39 ` Nautiyal, Ankit K
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2023-04-18 12:46 UTC (permalink / raw)
To: Manasi Navare; +Cc: intel-gfx
On Mon, Apr 17, 2023 at 03:48:12PM -0700, Manasi Navare wrote:
> Hi Ville,
>
> Could you suggest how to handle the intel_dp_link_compute_config()
> when the max_bpp is returned as 0, currently
> it just exits the loop and returns a -EINVAL and this triggers the DSC path.
> While we should be completely failing the modeset and encoder_config
> in this case instead of trying DSC, correct?
The DSC path needs to handle the bpp limits correctly:
1. Take the baseline limits already computed
2. Further restrict them based on sink/source DSC capabilities/etc.
3. Make sure the uncompressed bpp value chosen is between the min/max
>
> Manasi
>
> On Thu, Apr 13, 2023 at 8:23 AM Manasi Navare <navaremanasi@chromium.org> wrote:
> >
> > On Tue, Apr 11, 2023 at 10:22 PM Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote:
> > > > On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
> > > > <ville.syrjala@linux.intel.com> wrote:
> > > > >
> > > > > On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
> > > > > > In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
> > > > > > we return 0 instead of returning an err code of -EINVAL.
> > > > > > This throws off the logic in the calling function.
> > > > >
> > > > > What logic? The caller doesn't expect to get an error.
> > > >
> > > > If this returns a 0, we end up using limits.max_bpp = 0 and in
> > > > intel_dp_compute_link_config_wide(),
> > > > since max_bpp is 0, it exits this for loop:
> > > >
> > > > for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns
> > > > -EINVAL which then wrongly goes to enable DSC even when link BW is
> > > > sufficient without DSC.
> > >
> > > And how woud max_bpp<0 prevent that?
> > >
> > > The real problem seems to be that the DSC code totally
> > > ignores bpp limits.
> >
> > Hi Ville,
> >
> > So I see a few concerns/questions:
> > - Why is the Max bpp value 0 in intel_dp_max_bpp, is that a valid case
> > and how should our link configurations handle that case when max_bpp
> > is 0?
> > - This is happening in a bug I am looking at with HDMI PCON, @Ankit
> > Nautiyal have we ever seen something similar where max_bpp for HDMi
> > PCON
> > is returned 0?
> > - I dont think its a problem with DSC code, but rather
> > intel_dp_compute_link_config() outer for loop where we vary
> > from max_bpp to min_bpp and see if any bpp works with available link
> > bw, how should we handle this when max_bpp = 0 if you are saying thats
> > a valid case?
> > - In this patch if I return -EINVAL instead of 0, then atleast the
> > entire encoder_config will fail and that will fail the modeset, since
> > it assumes max_bpp cannot be 0
> >
> > Could you please help answer above concerns and how to handle max bpp
> > = 0 case if that is valid? This patch is simply making that invalid
> > resulting into modeset failure
> >
> > Manasi
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-18 12:46 ` Ville Syrjälä
@ 2023-04-18 14:39 ` Nautiyal, Ankit K
2023-04-18 14:54 ` Ville Syrjälä
0 siblings, 1 reply; 14+ messages in thread
From: Nautiyal, Ankit K @ 2023-04-18 14:39 UTC (permalink / raw)
To: Ville Syrjälä, Manasi Navare; +Cc: intel-gfx
On 4/18/2023 6:16 PM, Ville Syrjälä wrote:
> On Mon, Apr 17, 2023 at 03:48:12PM -0700, Manasi Navare wrote:
>> Hi Ville,
>>
>> Could you suggest how to handle the intel_dp_link_compute_config()
>> when the max_bpp is returned as 0, currently
>> it just exits the loop and returns a -EINVAL and this triggers the DSC path.
>> While we should be completely failing the modeset and encoder_config
>> in this case instead of trying DSC, correct?
> The DSC path needs to handle the bpp limits correctly:
> 1. Take the baseline limits already computed
> 2. Further restrict them based on sink/source DSC capabilities/etc.
> 3. Make sure the uncompressed bpp value chosen is between the min/max
I have some older patch to try similar thing [1]. We try to iterate over
bpc to find pipe_bpp in the limits, then try to find out compressed_bpp.
But if intel_dp_max_bpp returns 0, limits.max_bpp is set to 0, so we
discard this for dsc case and re-calculate the limits.max_bpp?
[1] https://patchwork.freedesktop.org/patch/519346/?series=111391&rev=3
>
>> Manasi
>>
>> On Thu, Apr 13, 2023 at 8:23 AM Manasi Navare <navaremanasi@chromium.org> wrote:
>>> On Tue, Apr 11, 2023 at 10:22 PM Ville Syrjälä
>>> <ville.syrjala@linux.intel.com> wrote:
>>>> On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote:
>>>>> On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
>>>>> <ville.syrjala@linux.intel.com> wrote:
>>>>>> On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
>>>>>>> In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
>>>>>>> we return 0 instead of returning an err code of -EINVAL.
>>>>>>> This throws off the logic in the calling function.
>>>>>> What logic? The caller doesn't expect to get an error.
>>>>> If this returns a 0, we end up using limits.max_bpp = 0 and in
>>>>> intel_dp_compute_link_config_wide(),
>>>>> since max_bpp is 0, it exits this for loop:
>>>>>
>>>>> for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns
>>>>> -EINVAL which then wrongly goes to enable DSC even when link BW is
>>>>> sufficient without DSC.
>>>> And how woud max_bpp<0 prevent that?
>>>>
>>>> The real problem seems to be that the DSC code totally
>>>> ignores bpp limits.
>>> Hi Ville,
>>>
>>> So I see a few concerns/questions:
>>> - Why is the Max bpp value 0 in intel_dp_max_bpp, is that a valid case
>>> and how should our link configurations handle that case when max_bpp
>>> is 0?
>>> - This is happening in a bug I am looking at with HDMI PCON, @Ankit
>>> Nautiyal have we ever seen something similar where max_bpp for HDMi
>>> PCON
>>> is returned 0?
>>> - I dont think its a problem with DSC code, but rather
>>> intel_dp_compute_link_config() outer for loop where we vary
>>> from max_bpp to min_bpp and see if any bpp works with available link
>>> bw, how should we handle this when max_bpp = 0 if you are saying thats
>>> a valid case?
>>> - In this patch if I return -EINVAL instead of 0, then atleast the
>>> entire encoder_config will fail and that will fail the modeset, since
>>> it assumes max_bpp cannot be 0
>>>
>>> Could you please help answer above concerns and how to handle max bpp
>>> = 0 case if that is valid? This patch is simply making that invalid
>>> resulting into modeset failure
>>>
>>> Manasi
>>>> --
>>>> Ville Syrjälä
>>>> Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-18 14:39 ` Nautiyal, Ankit K
@ 2023-04-18 14:54 ` Ville Syrjälä
2023-04-18 22:42 ` Manasi Navare
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2023-04-18 14:54 UTC (permalink / raw)
To: Nautiyal, Ankit K; +Cc: intel-gfx
On Tue, Apr 18, 2023 at 08:09:16PM +0530, Nautiyal, Ankit K wrote:
>
> On 4/18/2023 6:16 PM, Ville Syrjälä wrote:
> > On Mon, Apr 17, 2023 at 03:48:12PM -0700, Manasi Navare wrote:
> >> Hi Ville,
> >>
> >> Could you suggest how to handle the intel_dp_link_compute_config()
> >> when the max_bpp is returned as 0, currently
> >> it just exits the loop and returns a -EINVAL and this triggers the DSC path.
> >> While we should be completely failing the modeset and encoder_config
> >> in this case instead of trying DSC, correct?
> > The DSC path needs to handle the bpp limits correctly:
> > 1. Take the baseline limits already computed
> > 2. Further restrict them based on sink/source DSC capabilities/etc.
> > 3. Make sure the uncompressed bpp value chosen is between the min/max
>
> I have some older patch to try similar thing [1]. We try to iterate over
> bpc to find pipe_bpp in the limits, then try to find out compressed_bpp.
>
> But if intel_dp_max_bpp returns 0, limits.max_bpp is set to 0, so we
> discard this for dsc case and re-calculate the limits.max_bpp?
You shouldn't discard anything. DSC should take the already computed
limits and potentially just shrink them further based on DSC specific
constraints.
Or is there some weird case where DSC would allow lower/higher bpp
than what our uncompressed bpp limits declare?
>
>
> [1] https://patchwork.freedesktop.org/patch/519346/?series=111391&rev=3
>
> >
> >> Manasi
> >>
> >> On Thu, Apr 13, 2023 at 8:23 AM Manasi Navare <navaremanasi@chromium.org> wrote:
> >>> On Tue, Apr 11, 2023 at 10:22 PM Ville Syrjälä
> >>> <ville.syrjala@linux.intel.com> wrote:
> >>>> On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote:
> >>>>> On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
> >>>>> <ville.syrjala@linux.intel.com> wrote:
> >>>>>> On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
> >>>>>>> In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
> >>>>>>> we return 0 instead of returning an err code of -EINVAL.
> >>>>>>> This throws off the logic in the calling function.
> >>>>>> What logic? The caller doesn't expect to get an error.
> >>>>> If this returns a 0, we end up using limits.max_bpp = 0 and in
> >>>>> intel_dp_compute_link_config_wide(),
> >>>>> since max_bpp is 0, it exits this for loop:
> >>>>>
> >>>>> for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns
> >>>>> -EINVAL which then wrongly goes to enable DSC even when link BW is
> >>>>> sufficient without DSC.
> >>>> And how woud max_bpp<0 prevent that?
> >>>>
> >>>> The real problem seems to be that the DSC code totally
> >>>> ignores bpp limits.
> >>> Hi Ville,
> >>>
> >>> So I see a few concerns/questions:
> >>> - Why is the Max bpp value 0 in intel_dp_max_bpp, is that a valid case
> >>> and how should our link configurations handle that case when max_bpp
> >>> is 0?
> >>> - This is happening in a bug I am looking at with HDMI PCON, @Ankit
> >>> Nautiyal have we ever seen something similar where max_bpp for HDMi
> >>> PCON
> >>> is returned 0?
> >>> - I dont think its a problem with DSC code, but rather
> >>> intel_dp_compute_link_config() outer for loop where we vary
> >>> from max_bpp to min_bpp and see if any bpp works with available link
> >>> bw, how should we handle this when max_bpp = 0 if you are saying thats
> >>> a valid case?
> >>> - In this patch if I return -EINVAL instead of 0, then atleast the
> >>> entire encoder_config will fail and that will fail the modeset, since
> >>> it assumes max_bpp cannot be 0
> >>>
> >>> Could you please help answer above concerns and how to handle max bpp
> >>> = 0 case if that is valid? This patch is simply making that invalid
> >>> resulting into modeset failure
> >>>
> >>> Manasi
> >>>> --
> >>>> Ville Syrjälä
> >>>> Intel
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-18 14:54 ` Ville Syrjälä
@ 2023-04-18 22:42 ` Manasi Navare
2023-04-19 4:59 ` Nautiyal, Ankit K
0 siblings, 1 reply; 14+ messages in thread
From: Manasi Navare @ 2023-04-18 22:42 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Hi Ville and Ankit,
I actually do not think this is a problem with the DSC logic, but it
is a problem with the intel_dp_link_compute_config() where we should
do something if max_bpp is 0 instead of just returning a -EINVAL
directly.
My question here:
- In case of PCON, yuv format, is it a valid case to have max bpp set to 0?
- This is where I am seeing it as set to 0
- If it isnt then the problem is probably where it computes max bpp
for hdmi case for yuv format
Manasi
On Tue, Apr 18, 2023 at 7:54 AM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Tue, Apr 18, 2023 at 08:09:16PM +0530, Nautiyal, Ankit K wrote:
> >
> > On 4/18/2023 6:16 PM, Ville Syrjälä wrote:
> > > On Mon, Apr 17, 2023 at 03:48:12PM -0700, Manasi Navare wrote:
> > >> Hi Ville,
> > >>
> > >> Could you suggest how to handle the intel_dp_link_compute_config()
> > >> when the max_bpp is returned as 0, currently
> > >> it just exits the loop and returns a -EINVAL and this triggers the DSC path.
> > >> While we should be completely failing the modeset and encoder_config
> > >> in this case instead of trying DSC, correct?
> > > The DSC path needs to handle the bpp limits correctly:
> > > 1. Take the baseline limits already computed
> > > 2. Further restrict them based on sink/source DSC capabilities/etc.
> > > 3. Make sure the uncompressed bpp value chosen is between the min/max
> >
> > I have some older patch to try similar thing [1]. We try to iterate over
> > bpc to find pipe_bpp in the limits, then try to find out compressed_bpp.
> >
> > But if intel_dp_max_bpp returns 0, limits.max_bpp is set to 0, so we
> > discard this for dsc case and re-calculate the limits.max_bpp?
>
> You shouldn't discard anything. DSC should take the already computed
> limits and potentially just shrink them further based on DSC specific
> constraints.
>
> Or is there some weird case where DSC would allow lower/higher bpp
> than what our uncompressed bpp limits declare?
>
> >
> >
> > [1] https://patchwork.freedesktop.org/patch/519346/?series=111391&rev=3
> >
> > >
> > >> Manasi
> > >>
> > >> On Thu, Apr 13, 2023 at 8:23 AM Manasi Navare <navaremanasi@chromium.org> wrote:
> > >>> On Tue, Apr 11, 2023 at 10:22 PM Ville Syrjälä
> > >>> <ville.syrjala@linux.intel.com> wrote:
> > >>>> On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote:
> > >>>>> On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
> > >>>>> <ville.syrjala@linux.intel.com> wrote:
> > >>>>>> On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
> > >>>>>>> In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
> > >>>>>>> we return 0 instead of returning an err code of -EINVAL.
> > >>>>>>> This throws off the logic in the calling function.
> > >>>>>> What logic? The caller doesn't expect to get an error.
> > >>>>> If this returns a 0, we end up using limits.max_bpp = 0 and in
> > >>>>> intel_dp_compute_link_config_wide(),
> > >>>>> since max_bpp is 0, it exits this for loop:
> > >>>>>
> > >>>>> for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns
> > >>>>> -EINVAL which then wrongly goes to enable DSC even when link BW is
> > >>>>> sufficient without DSC.
> > >>>> And how woud max_bpp<0 prevent that?
> > >>>>
> > >>>> The real problem seems to be that the DSC code totally
> > >>>> ignores bpp limits.
> > >>> Hi Ville,
> > >>>
> > >>> So I see a few concerns/questions:
> > >>> - Why is the Max bpp value 0 in intel_dp_max_bpp, is that a valid case
> > >>> and how should our link configurations handle that case when max_bpp
> > >>> is 0?
> > >>> - This is happening in a bug I am looking at with HDMI PCON, @Ankit
> > >>> Nautiyal have we ever seen something similar where max_bpp for HDMi
> > >>> PCON
> > >>> is returned 0?
> > >>> - I dont think its a problem with DSC code, but rather
> > >>> intel_dp_compute_link_config() outer for loop where we vary
> > >>> from max_bpp to min_bpp and see if any bpp works with available link
> > >>> bw, how should we handle this when max_bpp = 0 if you are saying thats
> > >>> a valid case?
> > >>> - In this patch if I return -EINVAL instead of 0, then atleast the
> > >>> entire encoder_config will fail and that will fail the modeset, since
> > >>> it assumes max_bpp cannot be 0
> > >>>
> > >>> Could you please help answer above concerns and how to handle max bpp
> > >>> = 0 case if that is valid? This patch is simply making that invalid
> > >>> resulting into modeset failure
> > >>>
> > >>> Manasi
> > >>>> --
> > >>>> Ville Syrjälä
> > >>>> Intel
>
> --
> Ville Syrjälä
> Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-18 22:42 ` Manasi Navare
@ 2023-04-19 4:59 ` Nautiyal, Ankit K
2023-04-19 5:04 ` Nautiyal, Ankit K
0 siblings, 1 reply; 14+ messages in thread
From: Nautiyal, Ankit K @ 2023-04-19 4:59 UTC (permalink / raw)
To: Manasi Navare, Ville Syrjälä; +Cc: intel-gfx
On 4/19/2023 4:12 AM, Manasi Navare wrote:
> Hi Ville and Ankit,
>
> I actually do not think this is a problem with the DSC logic, but it
> is a problem with the intel_dp_link_compute_config() where we should
> do something if max_bpp is 0 instead of just returning a -EINVAL
> directly.
> My question here:
> - In case of PCON, yuv format, is it a valid case to have max bpp set to 0?
> - This is where I am seeing it as set to 0
> - If it isnt then the problem is probably where it computes max bpp
> for hdmi case for yuv format
I got your point. if limits.max_bpp is set to 0 (as given by
intel_dp_max_bpp)
and we cannot discard it as mentioned by Ville, then even with DSC we
cant do anything.
In such a case perhaps it makes sense to check if limit.max_bpp is 0 and
return -EINVAL from intel_dp_compute_link_config.
Regards,
Ankit
>
> Manasi
>
> On Tue, Apr 18, 2023 at 7:54 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>> On Tue, Apr 18, 2023 at 08:09:16PM +0530, Nautiyal, Ankit K wrote:
>>> On 4/18/2023 6:16 PM, Ville Syrjälä wrote:
>>>> On Mon, Apr 17, 2023 at 03:48:12PM -0700, Manasi Navare wrote:
>>>>> Hi Ville,
>>>>>
>>>>> Could you suggest how to handle the intel_dp_link_compute_config()
>>>>> when the max_bpp is returned as 0, currently
>>>>> it just exits the loop and returns a -EINVAL and this triggers the DSC path.
>>>>> While we should be completely failing the modeset and encoder_config
>>>>> in this case instead of trying DSC, correct?
>>>> The DSC path needs to handle the bpp limits correctly:
>>>> 1. Take the baseline limits already computed
>>>> 2. Further restrict them based on sink/source DSC capabilities/etc.
>>>> 3. Make sure the uncompressed bpp value chosen is between the min/max
>>> I have some older patch to try similar thing [1]. We try to iterate over
>>> bpc to find pipe_bpp in the limits, then try to find out compressed_bpp.
>>>
>>> But if intel_dp_max_bpp returns 0, limits.max_bpp is set to 0, so we
>>> discard this for dsc case and re-calculate the limits.max_bpp?
>> You shouldn't discard anything. DSC should take the already computed
>> limits and potentially just shrink them further based on DSC specific
>> constraints.
>>
>> Or is there some weird case where DSC would allow lower/higher bpp
>> than what our uncompressed bpp limits declare?
>>
>>>
>>> [1] https://patchwork.freedesktop.org/patch/519346/?series=111391&rev=3
>>>
>>>>> Manasi
>>>>>
>>>>> On Thu, Apr 13, 2023 at 8:23 AM Manasi Navare <navaremanasi@chromium.org> wrote:
>>>>>> On Tue, Apr 11, 2023 at 10:22 PM Ville Syrjälä
>>>>>> <ville.syrjala@linux.intel.com> wrote:
>>>>>>> On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote:
>>>>>>>> On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
>>>>>>>> <ville.syrjala@linux.intel.com> wrote:
>>>>>>>>> On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
>>>>>>>>>> In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error,
>>>>>>>>>> we return 0 instead of returning an err code of -EINVAL.
>>>>>>>>>> This throws off the logic in the calling function.
>>>>>>>>> What logic? The caller doesn't expect to get an error.
>>>>>>>> If this returns a 0, we end up using limits.max_bpp = 0 and in
>>>>>>>> intel_dp_compute_link_config_wide(),
>>>>>>>> since max_bpp is 0, it exits this for loop:
>>>>>>>>
>>>>>>>> for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns
>>>>>>>> -EINVAL which then wrongly goes to enable DSC even when link BW is
>>>>>>>> sufficient without DSC.
>>>>>>> And how woud max_bpp<0 prevent that?
>>>>>>>
>>>>>>> The real problem seems to be that the DSC code totally
>>>>>>> ignores bpp limits.
>>>>>> Hi Ville,
>>>>>>
>>>>>> So I see a few concerns/questions:
>>>>>> - Why is the Max bpp value 0 in intel_dp_max_bpp, is that a valid case
>>>>>> and how should our link configurations handle that case when max_bpp
>>>>>> is 0?
>>>>>> - This is happening in a bug I am looking at with HDMI PCON, @Ankit
>>>>>> Nautiyal have we ever seen something similar where max_bpp for HDMi
>>>>>> PCON
>>>>>> is returned 0?
>>>>>> - I dont think its a problem with DSC code, but rather
>>>>>> intel_dp_compute_link_config() outer for loop where we vary
>>>>>> from max_bpp to min_bpp and see if any bpp works with available link
>>>>>> bw, how should we handle this when max_bpp = 0 if you are saying thats
>>>>>> a valid case?
>>>>>> - In this patch if I return -EINVAL instead of 0, then atleast the
>>>>>> entire encoder_config will fail and that will fail the modeset, since
>>>>>> it assumes max_bpp cannot be 0
>>>>>>
>>>>>> Could you please help answer above concerns and how to handle max bpp
>>>>>> = 0 case if that is valid? This patch is simply making that invalid
>>>>>> resulting into modeset failure
>>>>>>
>>>>>> Manasi
>>>>>>> --
>>>>>>> Ville Syrjälä
>>>>>>> Intel
>> --
>> Ville Syrjälä
>> Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0
2023-04-19 4:59 ` Nautiyal, Ankit K
@ 2023-04-19 5:04 ` Nautiyal, Ankit K
0 siblings, 0 replies; 14+ messages in thread
From: Nautiyal, Ankit K @ 2023-04-19 5:04 UTC (permalink / raw)
To: Manasi Navare, Ville Syrjälä; +Cc: intel-gfx
On 4/19/2023 10:29 AM, Nautiyal, Ankit K wrote:
>
> On 4/19/2023 4:12 AM, Manasi Navare wrote:
>> Hi Ville and Ankit,
>>
>> I actually do not think this is a problem with the DSC logic, but it
>> is a problem with the intel_dp_link_compute_config() where we should
>> do something if max_bpp is 0 instead of just returning a -EINVAL
>> directly.
>> My question here:
>> - In case of PCON, yuv format, is it a valid case to have max bpp set
>> to 0?
>> - This is where I am seeing it as set to 0
>> - If it isnt then the problem is probably where it computes max bpp
>> for hdmi case for yuv format
>
> I got your point. if limits.max_bpp is set to 0 (as given by
> intel_dp_max_bpp)
>
> and we cannot discard it as mentioned by Ville, then even with DSC we
> cant do anything.
>
> In such a case perhaps it makes sense to check if limit.max_bpp is 0
> and return -EINVAL from intel_dp_compute_link_config.
Though just this change is not sufficient for that. I mean we will need
to handle in intel_dp_compute_link_config.
>
> Regards,
>
> Ankit
>
>>
>> Manasi
>>
>> On Tue, Apr 18, 2023 at 7:54 AM Ville Syrjälä
>> <ville.syrjala@linux.intel.com> wrote:
>>> On Tue, Apr 18, 2023 at 08:09:16PM +0530, Nautiyal, Ankit K wrote:
>>>> On 4/18/2023 6:16 PM, Ville Syrjälä wrote:
>>>>> On Mon, Apr 17, 2023 at 03:48:12PM -0700, Manasi Navare wrote:
>>>>>> Hi Ville,
>>>>>>
>>>>>> Could you suggest how to handle the intel_dp_link_compute_config()
>>>>>> when the max_bpp is returned as 0, currently
>>>>>> it just exits the loop and returns a -EINVAL and this triggers
>>>>>> the DSC path.
>>>>>> While we should be completely failing the modeset and encoder_config
>>>>>> in this case instead of trying DSC, correct?
>>>>> The DSC path needs to handle the bpp limits correctly:
>>>>> 1. Take the baseline limits already computed
>>>>> 2. Further restrict them based on sink/source DSC capabilities/etc.
>>>>> 3. Make sure the uncompressed bpp value chosen is between the min/max
>>>> I have some older patch to try similar thing [1]. We try to iterate
>>>> over
>>>> bpc to find pipe_bpp in the limits, then try to find out
>>>> compressed_bpp.
>>>>
>>>> But if intel_dp_max_bpp returns 0, limits.max_bpp is set to 0, so we
>>>> discard this for dsc case and re-calculate the limits.max_bpp?
>>> You shouldn't discard anything. DSC should take the already computed
>>> limits and potentially just shrink them further based on DSC specific
>>> constraints.
>>>
>>> Or is there some weird case where DSC would allow lower/higher bpp
>>> than what our uncompressed bpp limits declare?
>>>
>>>>
>>>> [1]
>>>> https://patchwork.freedesktop.org/patch/519346/?series=111391&rev=3
>>>>
>>>>>> Manasi
>>>>>>
>>>>>> On Thu, Apr 13, 2023 at 8:23 AM Manasi Navare
>>>>>> <navaremanasi@chromium.org> wrote:
>>>>>>> On Tue, Apr 11, 2023 at 10:22 PM Ville Syrjälä
>>>>>>> <ville.syrjala@linux.intel.com> wrote:
>>>>>>>> On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote:
>>>>>>>>> On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä
>>>>>>>>> <ville.syrjala@linux.intel.com> wrote:
>>>>>>>>>> On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote:
>>>>>>>>>>> In the function intel_dp_max_bpp(), currently if bpc < 0 in
>>>>>>>>>>> case of error,
>>>>>>>>>>> we return 0 instead of returning an err code of -EINVAL.
>>>>>>>>>>> This throws off the logic in the calling function.
>>>>>>>>>> What logic? The caller doesn't expect to get an error.
>>>>>>>>> If this returns a 0, we end up using limits.max_bpp = 0 and in
>>>>>>>>> intel_dp_compute_link_config_wide(),
>>>>>>>>> since max_bpp is 0, it exits this for loop:
>>>>>>>>>
>>>>>>>>> for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 *
>>>>>>>>> 3) and returns
>>>>>>>>> -EINVAL which then wrongly goes to enable DSC even when link
>>>>>>>>> BW is
>>>>>>>>> sufficient without DSC.
>>>>>>>> And how woud max_bpp<0 prevent that?
>>>>>>>>
>>>>>>>> The real problem seems to be that the DSC code totally
>>>>>>>> ignores bpp limits.
>>>>>>> Hi Ville,
>>>>>>>
>>>>>>> So I see a few concerns/questions:
>>>>>>> - Why is the Max bpp value 0 in intel_dp_max_bpp, is that a
>>>>>>> valid case
>>>>>>> and how should our link configurations handle that case when
>>>>>>> max_bpp
>>>>>>> is 0?
>>>>>>> - This is happening in a bug I am looking at with HDMI PCON, @Ankit
>>>>>>> Nautiyal have we ever seen something similar where max_bpp for
>>>>>>> HDMi
>>>>>>> PCON
>>>>>>> is returned 0?
>>>>>>> - I dont think its a problem with DSC code, but rather
>>>>>>> intel_dp_compute_link_config() outer for loop where we vary
>>>>>>> from max_bpp to min_bpp and see if any bpp works with available
>>>>>>> link
>>>>>>> bw, how should we handle this when max_bpp = 0 if you are saying
>>>>>>> thats
>>>>>>> a valid case?
>>>>>>> - In this patch if I return -EINVAL instead of 0, then atleast the
>>>>>>> entire encoder_config will fail and that will fail the modeset,
>>>>>>> since
>>>>>>> it assumes max_bpp cannot be 0
>>>>>>>
>>>>>>> Could you please help answer above concerns and how to handle
>>>>>>> max bpp
>>>>>>> = 0 case if that is valid? This patch is simply making that invalid
>>>>>>> resulting into modeset failure
>>>>>>>
>>>>>>> Manasi
>>>>>>>> --
>>>>>>>> Ville Syrjälä
>>>>>>>> Intel
>>> --
>>> Ville Syrjälä
>>> Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-04-19 5:04 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-11 17:34 [Intel-gfx] [PATCH] drm/i915/display: Return correct err code for bpc < 0 Manasi Navare
2023-04-11 17:42 ` Ville Syrjälä
2023-04-12 0:07 ` Manasi Navare
2023-04-12 5:22 ` Ville Syrjälä
2023-04-13 15:23 ` Manasi Navare
2023-04-17 22:48 ` Manasi Navare
2023-04-18 12:46 ` Ville Syrjälä
2023-04-18 14:39 ` Nautiyal, Ankit K
2023-04-18 14:54 ` Ville Syrjälä
2023-04-18 22:42 ` Manasi Navare
2023-04-19 4:59 ` Nautiyal, Ankit K
2023-04-19 5:04 ` Nautiyal, Ankit K
2023-04-12 0:28 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2023-04-12 13:44 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.