On Tue, 2025-12-09 at 12:38 +0200, Imre Deak wrote: > On Tue, Dec 09, 2025 at 10:47:35AM +0200, Jouni Hogander wrote: > > On Thu, 2025-11-27 at 19:50 +0200, Imre Deak wrote: > > > Move the initialization of the DSI DSC streams-per-pipe value to > > > fill_dsc() next to where the corresponding (per-line) slice_count > > > value > > > is initialized. This allows converting the initialization to use > > > the > > > detailed slice configuration state in follow-up changes. > > > > > > Signed-off-by: Imre Deak <[email protected]> > > > --- > > > drivers/gpu/drm/i915/display/icl_dsi.c | 6 ------ > > > drivers/gpu/drm/i915/display/intel_bios.c | 5 +++++ > > > 2 files changed, 5 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c > > > b/drivers/gpu/drm/i915/display/icl_dsi.c > > > index 90076839e7152..9aba3d813daae 100644 > > > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > > > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > > > @@ -1624,12 +1624,6 @@ static int > > > gen11_dsi_dsc_compute_config(struct > > > intel_encoder *encoder, > > > if (crtc_state->pipe_bpp < 8 * 3) > > > return -EINVAL; > > > > > > - /* FIXME: split only when necessary */ > > > - if (crtc_state->dsc.slice_count > 1) > > > - crtc_state->dsc.slice_config.streams_per_pipe = > > > 2; > > > - else > > > - crtc_state->dsc.slice_config.streams_per_pipe = > > > 1; > > > - > > > /* FIXME: initialize from VBT */ > > > vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST; > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > > > b/drivers/gpu/drm/i915/display/intel_bios.c > > > index a639c5eb32459..e69fac4f5bdfe 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_bios.c > > > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > > > @@ -3516,10 +3516,14 @@ static void fill_dsc(struct > > > intel_crtc_state *crtc_state, > > > * throughput etc. into account. > > > * > > > * Also, per spec DSI supports 1, 2, 3 or 4 horizontal > > > slices. > > > + * > > > + * FIXME: split only when necessary > > > */ > > > if (dsc->slices_per_line & BIT(2)) { > > > + crtc_state->dsc.slice_config.streams_per_pipe = > > > 2; > > > crtc_state->dsc.slice_count = 4; > > > } else if (dsc->slices_per_line & BIT(1)) { > > > + crtc_state->dsc.slice_config.streams_per_pipe = > > > 2; > > > > fill_dsc is called by intel_bios_get_dsc_params. Is > > streams_per_pipe > > really bios parameter? I see slices_per_line is in VBT. > > Streams_per_pipe and existing slice_count are decided based on > > that. > > The slices_per_line computed in fill_dsc() at the moment > (crtc_state->dsc.slice_count) is not exactly what is in VBT. VBT > indicates what slices_per_line counts the sink supports, not what the > selected slices_per_line count should be (which would be a single > integer parameter in VBT, not a mask). > > > Is that right place to make that decisions or should we leave that > > decision to caller of intel_bios_get_dsc_params? > > I think the computation of the slices_per_line value (for which the > sink's slice_per_line capability mask is only one criteria) should be > at > the same spot where the closely related pipes_per_line, > streams_per_pipe > and slices_per_stream are computed as well. In fact at the end of the > patchset only these latter 3 params are computed and the > slices_per_line > value is derived from these using a helper function. > > I agree with you that fill_dsc() should not do the actual state > computation (like it does atm selecting slices_per_line aka > dsc.slice_count), rather this should be done by the DSI encoder state > computation in gen11_dsi_dsc_compute_config(), fill_dsc() only > returning > a mask of the slices_per_line counts supported by the sink. Would you > be > ok to do this as a follow-up?
Thank you for the explanation. I'm fine with doing it as a follow-up: Reviewed-by: Jouni Högander <[email protected]> > > > BR, > > > > Jouni Högander > > > > > crtc_state->dsc.slice_count = 2; > > > } else { > > > /* FIXME */ > > > @@ -3527,6 +3531,7 @@ static void fill_dsc(struct > > > intel_crtc_state *crtc_state, > > > drm_dbg_kms(display->drm, > > > "VBT: Unsupported DSC slice > > > count for DSI\n"); > > > > > > + crtc_state->dsc.slice_config.streams_per_pipe = > > > 1; > > > crtc_state->dsc.slice_count = 1; > > > } > > >
