On Thu, Jan 02, 2025 at 12:52:38PM +0200, Jani Nikula wrote:
> On Tue, 31 Dec 2024, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> > On Thu, Dec 19, 2024 at 11:33:59PM +0200, Jani Nikula wrote:
> >> Write the payload allocation table for 128b/132b SST. Use VCPID 1 and
> >> start from slot 0, with dp_m_n.tu slots.
> >>
> >> This is preparation for enabling 128b/132b SST. This path is not
> >> reachable yet. Indeed, we don't yet compute TU for 128b/132b SST.
> >>
> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/display/intel_ddi.c | 11 +++++++++++
> >> 1 file changed, 11 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> >> index 6f813bf85b23..64528ff8856e 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> >> @@ -2669,6 +2669,12 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
> >> /* 6.o Configure and enable FEC if needed */
> >> intel_ddi_enable_fec(encoder, crtc_state);
> >>
> >> + /* 7.a 128b/132b SST. */
> >> + if (!is_mst && intel_dp_is_uhbr(crtc_state)) {
> >> + /* VCPID 1, start slot 0 for 128b/132b, tu slots */
> >> + drm_dp_dpcd_write_payload(&intel_dp->aux, 1, 0, crtc_state->dp_m_n.tu);
> >
> > I would handle the error by sending a modeset retry uevent. Either way:
>
> Mmmh. The MST code calls intel_dp_queue_modeset_retry_for_link() in
> response to drm_dp_add_payload_part{1,2}() failures, but nothing
> else. As in, we don't do anything else (we just plunge on with the
> enable), and we don't even do that much in response to any other errors.
>
> Our options are limited anyway.
The right thing to do is to report all these errors to user space, so it
can recover from it.
> Arguably the MST path payload update has more failure modes than SST,
> but you'd like to do this:
>
> ret = drm_dp_dpcd_write_payload(&intel_dp->aux, 1, 0, crtc_state->dp_m_n.tu);
> if (ret < 0)
> intel_dp_queue_modeset_retry_for_link(state, encoder, crtc_state);
>
> in mtl_ddi_pre_enable_dp() and tgl_ddi_pre_enable_dp()?
Yes.
> Overall I'm wondering if this too needs an overhaul. Instead of queueing
> the retry all over the place, maybe we should just flag it in
> crtc_state, and have intel_atomic_cleanup_work() do it once afterwards?
Atm it's queued once after detecting the first error, not sure if
deferring that would be better.
> BR,
> Jani.
>
> >
> > Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> >
> >> + }
> >> +
> >> if (!is_mst)
> >> intel_dsc_dp_pps_write(encoder, crtc_state);
> >> }
> >> @@ -2808,6 +2814,11 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
> >> /* 7.l Configure and enable FEC if needed */
> >> intel_ddi_enable_fec(encoder, crtc_state);
> >>
> >> + if (!is_mst && intel_dp_is_uhbr(crtc_state)) {
> >> + /* VCPID 1, start slot 0 for 128b/132b, tu slots */
> >> + drm_dp_dpcd_write_payload(&intel_dp->aux, 1, 0, crtc_state->dp_m_n.tu);
> >> + }
> >> +
> >> if (!is_mst)
> >> intel_dsc_dp_pps_write(encoder, crtc_state);
> >> }
> >> --
> >> 2.39.5
> >>
>
> --
> Jani Nikula, Intel
