| Message ID | 20210719072222.13369-6-shawn.c.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MIPI DSI driver enhancements | expand |
> -----Original Message----- > From: Lee, Shawn C <shawn.c.lee@intel.com> > Sent: Monday, July 19, 2021 12:52 PM > To: intel-gfx@lists.freedesktop.org > Cc: Lee, Shawn C <shawn.c.lee@intel.com>; Ville Syrjala > <ville.syrjala@linux.intel.com>; Jani Nikula <jani.nikula@linux.intel.com>; > Kulkarni, Vandita <vandita.kulkarni@intel.com>; Chiou, Cooper > <cooper.chiou@intel.com>; Tseng, William <william.tseng@intel.com> > Subject: [PATCH 5/5] drm/i915: Get proper min cdclk if vDSC enabled > > VDSC engine can process only 1 pixel per Cd clock. In case VDSC is used and > max slice count == 1, max supported pixel clock should be 100% of CD clock. > Then do min_cdclk and pixel clock comparison to get proper min cdclk. > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > Cc: Cooper Chiou <cooper.chiou@intel.com> > Cc: William Tseng <william.tseng@intel.com> > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 71067a62264d..c33d574eb991 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -2159,6 +2159,18 @@ int intel_crtc_compute_min_cdclk(const struct > intel_crtc_state *crtc_state) > /* Account for additional needs from the planes */ > min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk); > > + /* > + * VDSC engine can process only 1 pixel per Cd clock. > + * In case VDSC is used and max slice count == 1, > + * max supported pixel clock should be 100% of CD clock. > + * Then do min_cdclk and pixel clock comparison to get cdclk. > + */ > + if (DISPLAY_VER(dev_priv) >= 11 && I think you could just check for dsc enable and slice count ==1. Also better to have a check if crtc_clock exceeds dev_priv->max_cdclk_freq in dsi_dsc compute_config as well. and return -EINVAL . -Vandita > + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) && > + crtc_state->dsc.compression_enable && > + crtc_state->dsc.slice_count == 1) > + min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate); > + > /* > * HACK. Currently for TGL platforms we calculate > * min_cdclk initially based on pixel_rate divided > -- > 2.17.1
On Tue, July 19, 2021, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote: >> >> VDSC engine can process only 1 pixel per Cd clock. In case VDSC is >> used and max slice count == 1, max supported pixel clock should be 100% of CD clock. >> Then do min_cdclk and pixel clock comparison to get proper min cdclk. >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> >> Cc: Cooper Chiou <cooper.chiou@intel.com> >> Cc: William Tseng <william.tseng@intel.com> >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_cdclk.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c >> b/drivers/gpu/drm/i915/display/intel_cdclk.c >> index 71067a62264d..c33d574eb991 100644 >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c >> @@ -2159,6 +2159,18 @@ int intel_crtc_compute_min_cdclk(const struct >> intel_crtc_state *crtc_state) >> /* Account for additional needs from the planes */ min_cdclk = >> max(intel_planes_min_cdclk(crtc_state), min_cdclk); >> >> +/* >> + * VDSC engine can process only 1 pixel per Cd clock. >> + * In case VDSC is used and max slice count == 1, >> + * max supported pixel clock should be 100% of CD clock. >> + * Then do min_cdclk and pixel clock comparison to get cdclk. >> + */ >> +if (DISPLAY_VER(dev_priv) >= 11 && > >I think you could just check for dsc enable and slice count ==1. > DP and eDP would apply the same thing if dsc enabled and sink's dsc slice count ==1. Is that right? >Also better to have a check if crtc_clock exceeds dev_priv->max_cdclk_freq in dsi_dsc compute_config as well. >and return -EINVAL . > >-Vandita > We should have this checking in dsi_dsc_compute_config() just like DP driver did. What do you think? if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq || pipe_config->bigjoiner) { if (pipe_config->dsc.slice_count < 2) { drm_dbg_kms(&dev_priv->drm, "Cannot split stream to use 2 VDSC instances\n"); return -EINVAL; } pipe_config->dsc.dsc_split = true; } Best regards, Shawn >> + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) && >> + crtc_state->dsc.compression_enable && >> + crtc_state->dsc.slice_count == 1) > >> +min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate); >> + >> /* >> * HACK. Currently for TGL platforms we calculate >> * min_cdclk initially based on pixel_rate divided >> -- >> 2.17.1 > >
> -----Original Message----- > From: Lee, Shawn C <shawn.c.lee@intel.com> > Sent: Tuesday, July 20, 2021 8:01 PM > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>; Jani Nikula > <jani.nikula@linux.intel.com>; Chiou, Cooper <cooper.chiou@intel.com>; > Tseng, William <william.tseng@intel.com> > Subject: RE: [PATCH 5/5] drm/i915: Get proper min cdclk if vDSC enabled > > > On Tue, July 19, 2021, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote: > >> > >> VDSC engine can process only 1 pixel per Cd clock. In case VDSC is > >> used and max slice count == 1, max supported pixel clock should be 100% > of CD clock. > >> Then do min_cdclk and pixel clock comparison to get proper min cdclk. > >> > >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > >> Cc: Jani Nikula <jani.nikula@linux.intel.com> > >> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > >> Cc: Cooper Chiou <cooper.chiou@intel.com> > >> Cc: William Tseng <william.tseng@intel.com> > >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_cdclk.c | 12 ++++++++++++ > >> 1 file changed, 12 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > >> b/drivers/gpu/drm/i915/display/intel_cdclk.c > >> index 71067a62264d..c33d574eb991 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > >> @@ -2159,6 +2159,18 @@ int intel_crtc_compute_min_cdclk(const struct > >> intel_crtc_state *crtc_state) > >> /* Account for additional needs from the planes */ min_cdclk = > >> max(intel_planes_min_cdclk(crtc_state), min_cdclk); > >> > >> +/* > >> + * VDSC engine can process only 1 pixel per Cd clock. > >> + * In case VDSC is used and max slice count == 1, > >> + * max supported pixel clock should be 100% of CD clock. > >> + * Then do min_cdclk and pixel clock comparison to get cdclk. > >> + */ > >> +if (DISPLAY_VER(dev_priv) >= 11 && > > > >I think you could just check for dsc enable and slice count ==1. > > > > DP and eDP would apply the same thing if dsc enabled and sink's dsc slice > count ==1. > Is that right? Yes. > > >Also better to have a check if crtc_clock exceeds dev_priv->max_cdclk_freq > in dsi_dsc compute_config as well. > >and return -EINVAL . > > > >-Vandita Since we do not have bigjoiner support on dsi yet and the check wrt max_cdclk_freq is taken care in intel_crtc_compute_config. I think you can leave this for now. You do not need this clock check again in dsi_compute_config -Vandita > > > > We should have this checking in dsi_dsc_compute_config() just like DP driver > did. What do you think? > > if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq || > pipe_config->bigjoiner) { > if (pipe_config->dsc.slice_count < 2) { > drm_dbg_kms(&dev_priv->drm, > "Cannot split stream to use 2 VDSC > instances\n"); > return -EINVAL; > } > > pipe_config->dsc.dsc_split = true; > } > > Best regards, > Shawn > > >> + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) && > >> + crtc_state->dsc.compression_enable && > >> + crtc_state->dsc.slice_count == 1) > > > >> +min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate); > >> + > >> /* > >> * HACK. Currently for TGL platforms we calculate > >> * min_cdclk initially based on pixel_rate divided > >> -- > >> 2.17.1 > > > >
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 71067a62264d..c33d574eb991 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2159,6 +2159,18 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) /* Account for additional needs from the planes */ min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk); + /* + * VDSC engine can process only 1 pixel per Cd clock. + * In case VDSC is used and max slice count == 1, + * max supported pixel clock should be 100% of CD clock. + * Then do min_cdclk and pixel clock comparison to get cdclk. + */ + if (DISPLAY_VER(dev_priv) >= 11 && + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) && + crtc_state->dsc.compression_enable && + crtc_state->dsc.slice_count == 1) + min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate); + /* * HACK. Currently for TGL platforms we calculate * min_cdclk initially based on pixel_rate divided
VDSC engine can process only 1 pixel per Cd clock. In case VDSC is used and max slice count == 1, max supported pixel clock should be 100% of CD clock. Then do min_cdclk and pixel clock comparison to get proper min cdclk. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> Cc: Cooper Chiou <cooper.chiou@intel.com> Cc: William Tseng <william.tseng@intel.com> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> --- drivers/gpu/drm/i915/display/intel_cdclk.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)