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? 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
