* [RFC] drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
@ 2024-03-01 7:49 Charlton Lin
2024-03-01 9:07 ` [RFC] drm/i915/dp: Log message when limiting SST link rate Jani Nikula
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Charlton Lin @ 2024-03-01 7:49 UTC (permalink / raw)
To: intel-gfx
Cc: Charlton Lin, Ville Syrjälä, Khaled Almahallawy,
Sean Paul
Driver currently limits link rate up to HBR3 in SST mode. Log a
message with monitor vendor, product id, and MSTM_CAP to
help understand what monitors are being downgraded by this limit.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Khaled Almahallawy <khaled.almahallawy@intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Charlton Lin <charlton.lin@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 6ece2c563c7a..0b2d6d88fd37 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2437,6 +2437,25 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
false,
&limits);
+ if (intel_dp_max_common_rate(intel_dp) > limits.max_rate) {
+ u8 mstm_cap;
+ u32 panel_id = drm_edid_get_panel_id(&intel_dp->aux.ddc);
+ char vend[4];
+ u16 product_id;
+
+ drm_dbg_kms(&i915->drm,
+ "Limiting LR from max common rate %d to %d\n",
+ intel_dp_max_common_rate(intel_dp), limits.max_rate);
+
+ drm_edid_decode_panel_id(panel_id, vend, &product_id);
+
+ if (intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_12 &&
+ drm_dp_dpcd_readb(&intel_dp->aux, DP_MSTM_CAP, &mstm_cap) == 1)
+ drm_dbg_kms(&i915->drm,
+ "Manufacturer=%s Model=%x Sink MSTM_CAP=%x\n",
+ vend, product_id, mstm_cap);
+ }
+
if (!dsc_needed) {
/*
* Optimize for slow and wide for everything, because there are some
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC] drm/i915/dp: Log message when limiting SST link rate
2024-03-01 7:49 [RFC] drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Charlton Lin
@ 2024-03-01 9:07 ` Jani Nikula
2024-03-01 9:18 ` Ville Syrjälä
2024-03-01 14:04 ` ✗ Fi.CI.BAT: failure for drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patchwork
2024-04-02 17:26 ` [RFC] " Manasi Navare
2 siblings, 1 reply; 5+ messages in thread
From: Jani Nikula @ 2024-03-01 9:07 UTC (permalink / raw)
To: Charlton Lin, intel-gfx
Cc: Charlton Lin, Ville Syrjälä, Khaled Almahallawy,
Sean Paul
On Thu, 29 Feb 2024, Charlton Lin <charlton.lin@intel.com> wrote:
> Driver currently limits link rate up to HBR3 in SST mode. Log a
> message with monitor vendor, product id, and MSTM_CAP to
> help understand what monitors are being downgraded by this limit.
Any logging of the sink details should be done exactly once at detect
time. No matter what happens after that, there's no need to spam the
dmesg with duplicate information. If the information currently logged is
insufficient, please add it at detect, where it helps *all* debugging,
not just a single restricted case.
More details inline.
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Khaled Almahallawy <khaled.almahallawy@intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Charlton Lin <charlton.lin@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 6ece2c563c7a..0b2d6d88fd37 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2437,6 +2437,25 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> false,
> &limits);
>
> + if (intel_dp_max_common_rate(intel_dp) > limits.max_rate) {
If link training has failed and the max rate is limited due to that,
you'll hit this on all retraining attempts.
And, of course, this is hit at every single modeset for 128b/132b
capable displays that don't support MST. That's just excessive.
> + u8 mstm_cap;
> + u32 panel_id = drm_edid_get_panel_id(&intel_dp->aux.ddc);
That reads the entire EDID base block again in the middle of compute
config. A big no. We've also taken care to cache the EDID to avoid
duplicate reads otherwise.
Moreover, it ignores any EDID overrides etc.
> + char vend[4];
> + u16 product_id;
> +
> + drm_dbg_kms(&i915->drm,
> + "Limiting LR from max common rate %d to %d\n",
> + intel_dp_max_common_rate(intel_dp), limits.max_rate);
> +
> + drm_edid_decode_panel_id(panel_id, vend, &product_id);
> +
> + if (intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_12 &&
> + drm_dp_dpcd_readb(&intel_dp->aux, DP_MSTM_CAP, &mstm_cap) == 1)
We shouldn't add extra DPCD reads nilly willy either. This should be
debug logged once at detect. I've got a WIP series that should address
this [1], once I fix it.
> + drm_dbg_kms(&i915->drm,
> + "Manufacturer=%s Model=%x Sink MSTM_CAP=%x\n",
> + vend, product_id, mstm_cap);
Ville and I have discussed adding entire EDID debug logging at
drm_edid.c level, which would address a lot more things than this.
BR,
Jani.
[1] https://patchwork.freedesktop.org/series/129468/
> + }
> +
> if (!dsc_needed) {
> /*
> * Optimize for slow and wide for everything, because there are some
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] drm/i915/dp: Log message when limiting SST link rate
2024-03-01 9:07 ` [RFC] drm/i915/dp: Log message when limiting SST link rate Jani Nikula
@ 2024-03-01 9:18 ` Ville Syrjälä
0 siblings, 0 replies; 5+ messages in thread
From: Ville Syrjälä @ 2024-03-01 9:18 UTC (permalink / raw)
To: Jani Nikula; +Cc: Charlton Lin, intel-gfx, Khaled Almahallawy, Sean Paul
On Fri, Mar 01, 2024 at 11:07:43AM +0200, Jani Nikula wrote:
> On Thu, 29 Feb 2024, Charlton Lin <charlton.lin@intel.com> wrote:
> > Driver currently limits link rate up to HBR3 in SST mode. Log a
> > message with monitor vendor, product id, and MSTM_CAP to
> > help understand what monitors are being downgraded by this limit.
>
> Any logging of the sink details should be done exactly once at detect
> time. No matter what happens after that, there's no need to spam the
> dmesg with duplicate information. If the information currently logged is
> insufficient, please add it at detect, where it helps *all* debugging,
> not just a single restricted case.
>
> More details inline.
>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Khaled Almahallawy <khaled.almahallawy@intel.com>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Signed-off-by: Charlton Lin <charlton.lin@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 6ece2c563c7a..0b2d6d88fd37 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -2437,6 +2437,25 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> > false,
> > &limits);
> >
> > + if (intel_dp_max_common_rate(intel_dp) > limits.max_rate) {
>
> If link training has failed and the max rate is limited due to that,
> you'll hit this on all retraining attempts.
>
> And, of course, this is hit at every single modeset for 128b/132b
> capable displays that don't support MST. That's just excessive.
>
> > + u8 mstm_cap;
> > + u32 panel_id = drm_edid_get_panel_id(&intel_dp->aux.ddc);
>
> That reads the entire EDID base block again in the middle of compute
> config. A big no. We've also taken care to cache the EDID to avoid
> duplicate reads otherwise.
>
> Moreover, it ignores any EDID overrides etc.
>
> > + char vend[4];
> > + u16 product_id;
> > +
> > + drm_dbg_kms(&i915->drm,
> > + "Limiting LR from max common rate %d to %d\n",
> > + intel_dp_max_common_rate(intel_dp), limits.max_rate);
> > +
> > + drm_edid_decode_panel_id(panel_id, vend, &product_id);
> > +
> > + if (intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_12 &&
> > + drm_dp_dpcd_readb(&intel_dp->aux, DP_MSTM_CAP, &mstm_cap) == 1)
>
> We shouldn't add extra DPCD reads nilly willy either. This should be
> debug logged once at detect. I've got a WIP series that should address
> this [1], once I fix it.
Also the stone tablets say "thou shalt not touch the hardware
in atomic_check()/.compute_config()".
This stuff gets executed for every TEST_ONLY commit, which means it:
- potentially runs while the hardware is asleep
- should be relatively fast
>
> > + drm_dbg_kms(&i915->drm,
> > + "Manufacturer=%s Model=%x Sink MSTM_CAP=%x\n",
> > + vend, product_id, mstm_cap);
>
> Ville and I have discussed adding entire EDID debug logging at
> drm_edid.c level, which would address a lot more things than this.
>
> BR,
> Jani.
>
>
> [1] https://patchwork.freedesktop.org/series/129468/
>
>
>
> > + }
> > +
> > if (!dsc_needed) {
> > /*
> > * Optimize for slow and wide for everything, because there are some
>
> --
> Jani Nikula, Intel
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 5+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
2024-03-01 7:49 [RFC] drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Charlton Lin
2024-03-01 9:07 ` [RFC] drm/i915/dp: Log message when limiting SST link rate Jani Nikula
@ 2024-03-01 14:04 ` Patchwork
2024-04-02 17:26 ` [RFC] " Manasi Navare
2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2024-03-01 14:04 UTC (permalink / raw)
To: Charlton Lin; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 5350 bytes --]
== Series Details ==
Series: drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
URL : https://patchwork.freedesktop.org/series/130597/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_14376 -> Patchwork_130597v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_130597v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_130597v1, 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_130597v1/index.html
Participating hosts (42 -> 39)
------------------------------
Missing (3): bat-mtlp-8 bat-kbl-2 fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_130597v1:
### IGT changes ###
#### Possible regressions ####
* igt@dmabuf@all-tests@dma_fence:
- bat-arls-1: [PASS][1] -> [DMESG-FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14376/bat-arls-1/igt@dmabuf@all-tests@dma_fence.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/bat-arls-1/igt@dmabuf@all-tests@dma_fence.html
* igt@dmabuf@all-tests@sanitycheck:
- bat-arls-1: [PASS][3] -> [ABORT][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14376/bat-arls-1/igt@dmabuf@all-tests@sanitycheck.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/bat-arls-1/igt@dmabuf@all-tests@sanitycheck.html
Known issues
------------
Here are the changes found in Patchwork_130597v1 that come from known issues:
### CI changes ###
#### Issues hit ####
* boot:
- fi-tgl-1115g4: [PASS][5] -> [FAIL][6] ([i915#8293])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14376/fi-tgl-1115g4/boot.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/fi-tgl-1115g4/boot.html
- fi-cfl-8109u: [PASS][7] -> [FAIL][8] ([i915#8293])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14376/fi-cfl-8109u/boot.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/fi-cfl-8109u/boot.html
#### Possible fixes ####
* boot:
- fi-apl-guc: [FAIL][9] ([i915#8293]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14376/fi-apl-guc/boot.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/fi-apl-guc/boot.html
### IGT changes ###
#### Issues hit ####
* igt@gem_lmem_swapping@basic:
- fi-apl-guc: NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#4613]) +3 other tests skip
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/fi-apl-guc/igt@gem_lmem_swapping@basic.html
* igt@kms_hdmi_inject@inject-audio:
- fi-apl-guc: NOTRUN -> [SKIP][12] ([fdo#109271]) +17 other tests skip
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/fi-apl-guc/igt@kms_hdmi_inject@inject-audio.html
* igt@kms_pipe_crc_basic@nonblocking-crc:
- bat-dg2-11: NOTRUN -> [SKIP][13] ([i915#9197])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc.html
#### Possible fixes ####
* igt@i915_selftest@live@gt_lrc:
- bat-adlp-9: [INCOMPLETE][14] ([i915#9413]) -> [PASS][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14376/bat-adlp-9/igt@i915_selftest@live@gt_lrc.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/bat-adlp-9/igt@i915_selftest@live@gt_lrc.html
* igt@i915_selftest@live@hangcheck:
- {bat-rpls-3}: [DMESG-WARN][16] ([i915#5591]) -> [PASS][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14376/bat-rpls-3/igt@i915_selftest@live@hangcheck.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/bat-rpls-3/igt@i915_selftest@live@hangcheck.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
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#5591]: https://gitlab.freedesktop.org/drm/intel/issues/5591
[i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293
[i915#9197]: https://gitlab.freedesktop.org/drm/intel/issues/9197
[i915#9413]: https://gitlab.freedesktop.org/drm/intel/issues/9413
Build changes
-------------
* Linux: CI_DRM_14376 -> Patchwork_130597v1
CI-20190529: 20190529
CI_DRM_14376: 320f2374b2d52629a6a52c6c3059f02efd73c0a5 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7741: 7741
Patchwork_130597v1: 320f2374b2d52629a6a52c6c3059f02efd73c0a5 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
e2461da545ba drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130597v1/index.html
[-- Attachment #2: Type: text/html, Size: 6404 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
2024-03-01 7:49 [RFC] drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Charlton Lin
2024-03-01 9:07 ` [RFC] drm/i915/dp: Log message when limiting SST link rate Jani Nikula
2024-03-01 14:04 ` ✗ Fi.CI.BAT: failure for drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patchwork
@ 2024-04-02 17:26 ` Manasi Navare
2 siblings, 0 replies; 5+ messages in thread
From: Manasi Navare @ 2024-04-02 17:26 UTC (permalink / raw)
To: Charlton Lin
Cc: intel-gfx, Ville Syrjälä, Khaled Almahallawy, Sean Paul
Thanks Charlton for the patch.
I think in general it is a good idea to log this when the max rate is
dropped to HBR3 for SST case.
Please find my comments below,
On Thu, Feb 29, 2024 at 11:49 PM Charlton Lin <charlton.lin@intel.com> wrote:
>
> Driver currently limits link rate up to HBR3 in SST mode. Log a
> message with monitor vendor, product id, and MSTM_CAP to
> help understand what monitors are being downgraded by this limit.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Khaled Almahallawy <khaled.almahallawy@intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Charlton Lin <charlton.lin@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 6ece2c563c7a..0b2d6d88fd37 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2437,6 +2437,25 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> false,
> &limits);
>
> + if (intel_dp_max_common_rate(intel_dp) > limits.max_rate) {
> + u8 mstm_cap;
> + u32 panel_id = drm_edid_get_panel_id(&intel_dp->aux.ddc);
> + char vend[4];
> + u16 product_id;
> +
> + drm_dbg_kms(&i915->drm,
> + "Limiting LR from max common rate %d to %d\n",
We dont use LR acronym anywhere in the kernel for link rate, just say
link rate here.
Also I think would be good to log the reason why we are dropping this
to HBR3 or add a comment with a Todo
for this
Manasi
> + intel_dp_max_common_rate(intel_dp), limits.max_rate);
> +
> + drm_edid_decode_panel_id(panel_id, vend, &product_id);
> +
> + if (intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_12 &&
> + drm_dp_dpcd_readb(&intel_dp->aux, DP_MSTM_CAP, &mstm_cap) == 1)
> + drm_dbg_kms(&i915->drm,
> + "Manufacturer=%s Model=%x Sink MSTM_CAP=%x\n",
> + vend, product_id, mstm_cap);
> + }
> +
> if (!dsc_needed) {
> /*
> * Optimize for slow and wide for everything, because there are some
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-02 17:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-01 7:49 [RFC] drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Charlton Lin
2024-03-01 9:07 ` [RFC] drm/i915/dp: Log message when limiting SST link rate Jani Nikula
2024-03-01 9:18 ` Ville Syrjälä
2024-03-01 14:04 ` ✗ Fi.CI.BAT: failure for drm/i915/dp: Log message when limiting SST link rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patchwork
2024-04-02 17:26 ` [RFC] " Manasi Navare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox