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
