On 4/19/2023 4:12 AM, Manasi Navare wrote:
Hi Ville and Ankit,
I actually do not think this is a problem with the DSC logic, but it
is a problem with the intel_dp_link_compute_config() where we should
do something if max_bpp is 0 instead of just returning a -EINVAL
directly.
My question here:
- In case of PCON, yuv format, is it a valid case to have max bpp set to 0?
- This is where I am seeing it as set to 0
- If it isnt then the problem is probably where it computes max bpp
for hdmi case for yuv format
I got your point. if limits.max_bpp is set to 0 (as given by
intel_dp_max_bpp)
and we cannot discard it as mentioned by Ville, then even with DSC we
cant do anything.
In such a case perhaps it makes sense to check if limit.max_bpp is 0 and
return -EINVAL from intel_dp_compute_link_config.
Regards,
Ankit
Manasi
On Tue, Apr 18, 2023 at 7:54 AM Ville Syrjälä
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
On Tue, Apr 18, 2023 at 08:09:16PM +0530, Nautiyal, Ankit K wrote:
On 4/18/2023 6:16 PM, Ville Syrjälä wrote:
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
I have some older patch to try similar thing [1]. We try to iterate over
bpc to find pipe_bpp in the limits, then try to find out compressed_bpp.
But if intel_dp_max_bpp returns 0, limits.max_bpp is set to 0, so we
discard this for dsc case and re-calculate the limits.max_bpp?
You shouldn't discard anything. DSC should take the already computed
limits and potentially just shrink them further based on DSC specific
constraints.
Or is there some weird case where DSC would allow lower/higher bpp
than what our uncompressed bpp limits declare?
[1] https://patchwork.freedesktop.org/patch/519346/?series=111391&rev=3
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
