[Intel-gfx] [PATCH 2/2] drm/i915/dp: Use current cdclk for DSC Bigjoiner BW check

Manasi Navare navaremanasi at google.com
Mon Apr 3 21:33:03 UTC 2023


On Thu, Mar 30, 2023 at 4:11 AM Nautiyal, Ankit K
<ankit.k.nautiyal at intel.com> wrote:
>
>
> On 3/29/2023 5:05 PM, Ville Syrjälä wrote:
> > On Wed, Mar 29, 2023 at 05:00:55PM +0530, Nautiyal, Ankit K wrote:
> >> On 3/29/2023 4:23 PM, Ville Syrjälä wrote:
> >>> On Wed, Mar 29, 2023 at 04:06:21PM +0530, Nautiyal, Ankit K wrote:
> >>>> On 3/29/2023 3:27 PM, Ville Syrjälä wrote:
> >>>>> On Wed, Mar 29, 2023 at 02:14:49PM +0530, Ankit Nautiyal wrote:
> >>>>>> As per Bspec, Big Joiner BW check is:
> >>>>>> Output bpp <= PPC * CDCLK frequency * Big joiner interface bits /
> >>>>>> Pixel clock
> >>>>>>
> >>>>>> Currently we always use max_cdclk in the check for both modevalid
> >>>>>> and compute config steps.
> >>>>>>
> >>>>>> During modevalid use max_cdclk_freq for the check.
> >>>>>> During compute config step use current cdclk for the check.
> >>>>> Nak. cdclk is computed much later based on what is actually needed.
> >>>>> The cdclk freq you are using here is essentially a random number.
> >>>> Oh I didn't realise that, perhaps I was lucky when I tested this.
> >>>>
> >>>> So this check where CDCLK is mentioned, actually expects max_cdclk_freq?
> >>>>
> >>>> If it doesnt then, we might have a compressed_bpp value, that might be
> >>>> violating the big joiner bw check.
> >>>>
> >>>> Should this be handled while computing cdclk?
> >>> Yes. I suggest adding something like intel_vdsc_min_cdclk() that
> >>> handles all of it.
> >>
> >> I can try that out.
> >>
> >> Will also add *Pipe BW check*: Pixel clock < PPC * CDCLK frequency *
> >> Number of pipes joined, which seems to be missing.
> > That is already accounted for in the pixel rate.
>
> Indeed, will club this check and the other bigjoiner_bw check in
> intel_vdsc_min_cdclk, if the approach mentioned in the other mail is
> acceptable.
>
> Regards,
>
> Ankit

Hi Ankit,

Yes I think it makes sense to add the vdsc_min_cdclk check while
computing the min cdclk.
This will hopefully let us exercise all allowed compressed bpps all
the way to 27, which have caused failures earlier
possibly due to the cdcclk being reduced beyond the pipe bw required
with the max compressed bpp that we use.
So best would be to check against vdsc_mind_cdclk required if dsc is
enabled for that configuration, if not then skip
the check

Regards
Manasi
>
> >
> >> So with pipe bw_check cdclk should be >  Pixel clock / (PPC * Number of
> >> pipes joined)
> >>
> >> In addition, as per bigjoiner check it should be >= compressed_bpp /
> >> (PPC * bigjoiner interface bits).
> >>
> >> Regards,
> >>
> >> Ankit
> >>
> >>>> Regards,
> >>>>
> >>>> Ankit
> >>>>
> >>>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> >>>>>> ---
> >>>>>>     drivers/gpu/drm/i915/display/intel_dp.c     | 9 ++++++---
> >>>>>>     drivers/gpu/drm/i915/display/intel_dp.h     | 1 +
> >>>>>>     drivers/gpu/drm/i915/display/intel_dp_mst.c | 1 +
> >>>>>>     3 files changed, 8 insertions(+), 3 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >>>>>> index 3fe651a8f5d0..d6600de1ab49 100644
> >>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >>>>>> @@ -711,6 +711,7 @@ u32 intel_dp_dsc_nearest_valid_bpp(struct drm_i915_private *i915, u32 bpp, u32 p
> >>>>>>     u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
> >>>>>>                                  u32 link_clock, u32 lane_count,
> >>>>>>                                  u32 mode_clock, u32 mode_hdisplay,
> >>>>>> +                                unsigned int cdclk,
> >>>>>>                                  bool bigjoiner,
> >>>>>>                                  u32 pipe_bpp,
> >>>>>>                                  u32 timeslots)
> >>>>>> @@ -757,9 +758,9 @@ u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
> >>>>>>
> >>>>>>          if (bigjoiner) {
> >>>>>>                  int bigjoiner_interface_bits = DISPLAY_VER(i915) <= 12 ? 24 : 36;
> >>>>>> -                u32 max_bpp_bigjoiner =
> >>>>>> -                        i915->display.cdclk.max_cdclk_freq * 2 * bigjoiner_interface_bits /
> >>>>>> -                        intel_dp_mode_to_fec_clock(mode_clock);
> >>>>>> +
> >>>>>> +                u32 max_bpp_bigjoiner = cdclk * 2 * bigjoiner_interface_bits /
> >>>>>> +                                        intel_dp_mode_to_fec_clock(mode_clock);
> >>>>>>
> >>>>>>                  bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);
> >>>>>>          }
> >>>>>> @@ -1073,6 +1074,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> >>>>>>                                                              max_lanes,
> >>>>>>                                                              target_clock,
> >>>>>>                                                              mode->hdisplay,
> >>>>>> +                                                            dev_priv->display.cdclk.max_cdclk_freq,
> >>>>>>                                                              bigjoiner,
> >>>>>>                                                              pipe_bpp, 64) >> 4;
> >>>>>>                          dsc_slice_count =
> >>>>>> @@ -1580,6 +1582,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> >>>>>>                                                              pipe_config->lane_count,
> >>>>>>                                                              adjusted_mode->crtc_clock,
> >>>>>>                                                              adjusted_mode->crtc_hdisplay,
> >>>>>> +                                                            dev_priv->display.cdclk.hw.cdclk,
> >>>>>>                                                              pipe_config->bigjoiner_pipes,
> >>>>>>                                                              pipe_bpp,
> >>>>>>                                                              timeslots);
> >>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> >>>>>> index ef39e4f7a329..d150bfe8abf4 100644
> >>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> >>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> >>>>>> @@ -106,6 +106,7 @@ int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc);
> >>>>>>     u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
> >>>>>>                                  u32 link_clock, u32 lane_count,
> >>>>>>                                  u32 mode_clock, u32 mode_hdisplay,
> >>>>>> +                                unsigned int cdclk,
> >>>>>>                                  bool bigjoiner,
> >>>>>>                                  u32 pipe_bpp,
> >>>>>>                                  u32 timeslots);
> >>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >>>>>> index a860cbc5dbea..266e31b78729 100644
> >>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >>>>>> @@ -925,6 +925,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
> >>>>>>                                                              max_lanes,
> >>>>>>                                                              target_clock,
> >>>>>>                                                              mode->hdisplay,
> >>>>>> +                                                            dev_priv->display.cdclk.max_cdclk_freq,
> >>>>>>                                                              bigjoiner,
> >>>>>>                                                              pipe_bpp, 64) >> 4;
> >>>>>>                          dsc_slice_count =
> >>>>>> --
> >>>>>> 2.25.1


More information about the Intel-gfx mailing list