> -----Original Message----- > From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > Sent: Wednesday, May 17, 2023 5:33 AM > To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; David Airlie > <airlied@xxxxxxxxx>; Daniel Vetter <daniel@xxxxxxxx>; Jani Nikula > <jani.nikula@xxxxxxxxxxxxxxx>; Joonas Lahtinen > <joonas.lahtinen@xxxxxxxxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; > Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>; Rob Clark > <robdclark@xxxxxxxxx>; Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>; > Sean Paul <sean@xxxxxxxxxx>; Marijn Suijten > <marijn.suijten@xxxxxxxxxxxxxx> > Cc: linux-arm-msm@xxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > freedreno@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; Ville > Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Subject: Re: [Freedreno] [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and > DSC 1.1 (pre-SCR) parameters > > On 16/05/2023 21:46, Kandpal, Suraj wrote: > >> > >> The array of rc_parameters contains a mixture of parameters from DSC > >> 1.1 and DSC 1.2 standards. Split these tow configuration arrays in > >> preparation to adding more configuration data. > >> > > > > Hi , > > Needed to add some more comments apart from the previous ones > already > > given > > > > [skipped] > > > >> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c > >> b/drivers/gpu/drm/i915/display/intel_vdsc.c > >> index d4340b18c18d..bd9116d2cd76 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > >> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > >> @@ -226,7 +226,15 @@ int intel_dsc_compute_params(struct > >> intel_crtc_state *pipe_config) > >> if (DISPLAY_VER(dev_priv) >= 13) { > >> calculate_rc_params(vdsc_cfg); > >> } else { > >> - ret = drm_dsc_setup_rc_params(vdsc_cfg); > >> + if ((compressed_bpp == 8 || > >> + compressed_bpp == 12) && > >> + (vdsc_cfg->bits_per_component == 8 || > >> + vdsc_cfg->bits_per_component == 10 || > >> + vdsc_cfg->bits_per_component == 12)) > >> + ret = drm_dsc_setup_rc_params(vdsc_cfg, > >> DRM_DSC_1_1_PRE_SCR); > >> + else > >> + ret = drm_dsc_setup_rc_params(vdsc_cfg, > >> DRM_DSC_1_2_444); > >> + > > > > I do not think this kind of assignment works as you will also be > > adding > > DRM_DSC_1_2_422 and DRM_DSC_1_2_420 in further patches and AFAICS > > There is no where in patch 8 that you have accounted for when 422 or 420 > will be used. > > Maybe you can add an if case inside the else block to check > > pipe_config->output_format to pass the rc_param_data in patch 8 > > I don't think this is necessary for now. The driver doesn't support YUV 422. > The YUV 420 is supported only for DISPLAY_VER(dev_priv) >= 14, however > these helpers are only used for DISPLAY_VER(dev_priv) < 13. > > I did not move RC calculation to drm_dsc_helpers.c (yet ?), which is used for > DISPLAY_VER >= 13. Hmm. I see I'll work on it once this patch series is merged Regards, Suraj Kandpal > > > > > Regards, > > Suraj Kandpal > >> if (ret) > >> return ret; > >> > >> diff --git a/include/drm/display/drm_dsc_helper.h > >> b/include/drm/display/drm_dsc_helper.h > >> index 1681791f65a5..c634bb2935d3 100644 > >> --- a/include/drm/display/drm_dsc_helper.h > >> +++ b/include/drm/display/drm_dsc_helper.h > >> @@ -10,12 +10,17 @@ > >> > >> #include <drm/display/drm_dsc.h> > >> > >> +enum drm_dsc_params_kind { > >> + DRM_DSC_1_2_444, > >> + DRM_DSC_1_1_PRE_SCR, /* legacy params from DSC 1.1 */ }; > >> + > >> void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header); > >> int > >> drm_dsc_dp_rc_buffer_size(u8 rc_buffer_block_size, u8 > >> rc_buffer_size); void drm_dsc_pps_payload_pack(struct > >> drm_dsc_picture_parameter_set *pps_sdp, > >> const struct drm_dsc_config *dsc_cfg); void > >> drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg); -int > >> drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg); > >> +int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum > >> +drm_dsc_params_kind kind); > >> int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); > >> > >> #endif /* _DRM_DSC_HELPER_H_ */ > >> -- > >> 2.39.2 > > > > -- > With best wishes > Dmitry