| Message ID | 20240220211841.448846-4-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add Display Port tunnel BW allocation support | expand |
> -----Original Message----- > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Imre > Deak > Sent: Wednesday, February 21, 2024 2:48 AM > To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Subject: [PATCH v2 03/21] drm/i915: Fix display bpp limit computation during > system resume > > The system resume display mode restoration should happen with an output > configuration matching that of the suspend time saved mode. Since the restored > mode configuration is subject to the bpp fallback logic, starting out with an > unlimited bpp and reducing the bpp as required by any (MST) link BW limit, the > resulting bpp will match the one during suspend only if the BW limit checks during > suspend and resume are applied in an identical way. The latter is not guaranteed > at the moment, since the pre-suspend MST topology may not be in place during > resume (for instance if the MST sink was disconnected while being suspended), > which makes the MST link BW check accept the unlimited bpp mode > configuration unconditionally without ensuring that the required BW fits into the > available MST link BW. > > To fix the above, initialize the bpp fallback logic with the max link bpp / force-FEC > limits left behind by the suspend time mode save. Looks Good to me. Reviewed-by: Uma Shankar <uma.shankar@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 3 +-- > drivers/gpu/drm/i915/display/intel_link_bw.c | 22 ++++++++++++++++---- > drivers/gpu/drm/i915/display/intel_link_bw.h | 2 +- > 3 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 00ac65a140298..485c38d71f106 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -6252,12 +6252,11 @@ static int intel_atomic_check_config(struct > intel_atomic_state *state, > > static int intel_atomic_check_config_and_link(struct intel_atomic_state *state) { > - struct drm_i915_private *i915 = to_i915(state->base.dev); > struct intel_link_bw_limits new_limits; > struct intel_link_bw_limits old_limits; > int ret; > > - intel_link_bw_init_limits(i915, &new_limits); > + intel_link_bw_init_limits(state, &new_limits); > old_limits = new_limits; > > while (true) { > diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.c > b/drivers/gpu/drm/i915/display/intel_link_bw.c > index 9c6d35a405a18..27ea858897c9f 100644 > --- a/drivers/gpu/drm/i915/display/intel_link_bw.c > +++ b/drivers/gpu/drm/i915/display/intel_link_bw.c > @@ -6,6 +6,7 @@ > #include "i915_drv.h" > > #include "intel_atomic.h" > +#include "intel_crtc.h" > #include "intel_display_types.h" > #include "intel_dp_mst.h" > #include "intel_fdi.h" > @@ -13,19 +14,32 @@ > > /** > * intel_link_bw_init_limits - initialize BW limits > - * @i915: device instance > + * @state: Atomic state > * @limits: link BW limits > * > * Initialize @limits. > */ > -void intel_link_bw_init_limits(struct drm_i915_private *i915, struct > intel_link_bw_limits *limits) > +void intel_link_bw_init_limits(struct intel_atomic_state *state, > + struct intel_link_bw_limits *limits) > { > + struct drm_i915_private *i915 = to_i915(state->base.dev); > enum pipe pipe; > > limits->force_fec_pipes = 0; > limits->bpp_limit_reached_pipes = 0; > - for_each_pipe(i915, pipe) > - limits->max_bpp_x16[pipe] = INT_MAX; > + for_each_pipe(i915, pipe) { > + const struct intel_crtc_state *crtc_state = > + intel_atomic_get_new_crtc_state(state, > + > intel_crtc_for_pipe(i915, pipe)); > + > + if (state->base.duplicated && crtc_state) { > + limits->max_bpp_x16[pipe] = crtc_state- > >max_link_bpp_x16; > + if (crtc_state->fec_enable) > + limits->force_fec_pipes |= BIT(pipe); > + } else { > + limits->max_bpp_x16[pipe] = INT_MAX; > + } > + } > } > > /** > diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.h > b/drivers/gpu/drm/i915/display/intel_link_bw.h > index 2cf57307cc249..6b0ccfff59dab 100644 > --- a/drivers/gpu/drm/i915/display/intel_link_bw.h > +++ b/drivers/gpu/drm/i915/display/intel_link_bw.h > @@ -22,7 +22,7 @@ struct intel_link_bw_limits { > int max_bpp_x16[I915_MAX_PIPES]; > }; > > -void intel_link_bw_init_limits(struct drm_i915_private *i915, > +void intel_link_bw_init_limits(struct intel_atomic_state *state, > struct intel_link_bw_limits *limits); int > intel_link_bw_reduce_bpp(struct intel_atomic_state *state, > struct intel_link_bw_limits *limits, > -- > 2.39.2
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 00ac65a140298..485c38d71f106 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6252,12 +6252,11 @@ static int intel_atomic_check_config(struct intel_atomic_state *state, static int intel_atomic_check_config_and_link(struct intel_atomic_state *state) { - struct drm_i915_private *i915 = to_i915(state->base.dev); struct intel_link_bw_limits new_limits; struct intel_link_bw_limits old_limits; int ret; - intel_link_bw_init_limits(i915, &new_limits); + intel_link_bw_init_limits(state, &new_limits); old_limits = new_limits; while (true) { diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.c b/drivers/gpu/drm/i915/display/intel_link_bw.c index 9c6d35a405a18..27ea858897c9f 100644 --- a/drivers/gpu/drm/i915/display/intel_link_bw.c +++ b/drivers/gpu/drm/i915/display/intel_link_bw.c @@ -6,6 +6,7 @@ #include "i915_drv.h" #include "intel_atomic.h" +#include "intel_crtc.h" #include "intel_display_types.h" #include "intel_dp_mst.h" #include "intel_fdi.h" @@ -13,19 +14,32 @@ /** * intel_link_bw_init_limits - initialize BW limits - * @i915: device instance + * @state: Atomic state * @limits: link BW limits * * Initialize @limits. */ -void intel_link_bw_init_limits(struct drm_i915_private *i915, struct intel_link_bw_limits *limits) +void intel_link_bw_init_limits(struct intel_atomic_state *state, + struct intel_link_bw_limits *limits) { + struct drm_i915_private *i915 = to_i915(state->base.dev); enum pipe pipe; limits->force_fec_pipes = 0; limits->bpp_limit_reached_pipes = 0; - for_each_pipe(i915, pipe) - limits->max_bpp_x16[pipe] = INT_MAX; + for_each_pipe(i915, pipe) { + const struct intel_crtc_state *crtc_state = + intel_atomic_get_new_crtc_state(state, + intel_crtc_for_pipe(i915, pipe)); + + if (state->base.duplicated && crtc_state) { + limits->max_bpp_x16[pipe] = crtc_state->max_link_bpp_x16; + if (crtc_state->fec_enable) + limits->force_fec_pipes |= BIT(pipe); + } else { + limits->max_bpp_x16[pipe] = INT_MAX; + } + } } /** diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.h b/drivers/gpu/drm/i915/display/intel_link_bw.h index 2cf57307cc249..6b0ccfff59dab 100644 --- a/drivers/gpu/drm/i915/display/intel_link_bw.h +++ b/drivers/gpu/drm/i915/display/intel_link_bw.h @@ -22,7 +22,7 @@ struct intel_link_bw_limits { int max_bpp_x16[I915_MAX_PIPES]; }; -void intel_link_bw_init_limits(struct drm_i915_private *i915, +void intel_link_bw_init_limits(struct intel_atomic_state *state, struct intel_link_bw_limits *limits); int intel_link_bw_reduce_bpp(struct intel_atomic_state *state, struct intel_link_bw_limits *limits,
The system resume display mode restoration should happen with an output configuration matching that of the suspend time saved mode. Since the restored mode configuration is subject to the bpp fallback logic, starting out with an unlimited bpp and reducing the bpp as required by any (MST) link BW limit, the resulting bpp will match the one during suspend only if the BW limit checks during suspend and resume are applied in an identical way. The latter is not guaranteed at the moment, since the pre-suspend MST topology may not be in place during resume (for instance if the MST sink was disconnected while being suspended), which makes the MST link BW check accept the unlimited bpp mode configuration unconditionally without ensuring that the required BW fits into the available MST link BW. To fix the above, initialize the bpp fallback logic with the max link bpp / force-FEC limits left behind by the suspend time mode save. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 3 +-- drivers/gpu/drm/i915/display/intel_link_bw.c | 22 ++++++++++++++++---- drivers/gpu/drm/i915/display/intel_link_bw.h | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-)