On Mon, Apr 17, 2023 at 03:48:12PM -0700, Manasi Navare wrote: > Hi Ville, > > Could you suggest how to handle the intel_dp_link_compute_config() > when the max_bpp is returned as 0, currently > it just exits the loop and returns a -EINVAL and this triggers the DSC path. > While we should be completely failing the modeset and encoder_config > in this case instead of trying DSC, correct? The DSC path needs to handle the bpp limits correctly: 1. Take the baseline limits already computed 2. Further restrict them based on sink/source DSC capabilities/etc. 3. Make sure the uncompressed bpp value chosen is between the min/max > > Manasi > > On Thu, Apr 13, 2023 at 8:23 AM Manasi Navare <navaremanasi@xxxxxxxxxxxx> wrote: > > > > On Tue, Apr 11, 2023 at 10:22 PM Ville Syrjälä > > <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > > > > > On Tue, Apr 11, 2023 at 05:07:01PM -0700, Manasi Navare wrote: > > > > On Tue, Apr 11, 2023 at 10:42 AM Ville Syrjälä > > > > <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > > > > > > > > > On Tue, Apr 11, 2023 at 05:34:08PM +0000, Manasi Navare wrote: > > > > > > In the function intel_dp_max_bpp(), currently if bpc < 0 in case of error, > > > > > > we return 0 instead of returning an err code of -EINVAL. > > > > > > This throws off the logic in the calling function. > > > > > > > > > > What logic? The caller doesn't expect to get an error. > > > > > > > > If this returns a 0, we end up using limits.max_bpp = 0 and in > > > > intel_dp_compute_link_config_wide(), > > > > since max_bpp is 0, it exits this for loop: > > > > > > > > for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) and returns > > > > -EINVAL which then wrongly goes to enable DSC even when link BW is > > > > sufficient without DSC. > > > > > > And how woud max_bpp<0 prevent that? > > > > > > The real problem seems to be that the DSC code totally > > > ignores bpp limits. > > > > Hi Ville, > > > > So I see a few concerns/questions: > > - Why is the Max bpp value 0 in intel_dp_max_bpp, is that a valid case > > and how should our link configurations handle that case when max_bpp > > is 0? > > - This is happening in a bug I am looking at with HDMI PCON, @Ankit > > Nautiyal have we ever seen something similar where max_bpp for HDMi > > PCON > > is returned 0? > > - I dont think its a problem with DSC code, but rather > > intel_dp_compute_link_config() outer for loop where we vary > > from max_bpp to min_bpp and see if any bpp works with available link > > bw, how should we handle this when max_bpp = 0 if you are saying thats > > a valid case? > > - In this patch if I return -EINVAL instead of 0, then atleast the > > entire encoder_config will fail and that will fail the modeset, since > > it assumes max_bpp cannot be 0 > > > > Could you please help answer above concerns and how to handle max bpp > > = 0 case if that is valid? This patch is simply making that invalid > > resulting into modeset failure > > > > Manasi > > > > > > -- > > > Ville Syrjälä > > > Intel -- Ville Syrjälä Intel
