[PATCH] drm/omapdrm: dispc: Refuse x-decimation above 4 for all but 8-bit formats

[PATCH] drm/omapdrm: dispc: Refuse x-decimation above 4 for all but 8-bit formats

[Jyri Sarha]

Let's disable all scaling that requires horizontal decimation with
higher factor than 4, until we have better estimates of what we can
and can not do. However, 1 byte per pixel color format appear to work
Ok with all decimation factors.

When decimating horizontally by more that 4 the dss is not able to
fetch the data in burst mode. When this happens it is hard to tell if
there enough bandwidth. Despite what theory says this appears to be
true also for 16-bit color formats.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 5554b72..61daef6 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -2506,6 +2506,25 @@ static int dispc_ovl_calc_scaling_44xx(unsigned long pclk, unsigned long lclk,
 		return -EINVAL;
 	}
 
+	if (*decim_x > 4 && color_mode_to_bpp(color_mode) > 8) {
+		/*
+		  Let's disable all scaling that requires horizontal
+		  decimation with higher factor than 4, until we have
+		  better estimates of what we can and can not
+		  do. However, 1 byte per pixel color format appear to
+		  work Ok with all decimation factors.
+
+		  When decimating horizontally by more that 4 the dss
+		  is not able to fetch the data in burst mode. When
+		  this happens it is hard to tell if there enough
+		  bandwidth. Despite what theory says this appears to
+		  be true also for 16-bit color formats.
+		*/
+		DSSERR("Not enough bandwidth (x-decimation factor %d > 4)",
+			*decim_x);
+		return -EINVAL;
+	}
+
 	*core_clk = dispc.feat->calc_core_clk(pclk, in_width, in_height,
 				out_width, out_height, mem_to_mem);
 	return 0;
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omapdrm: dispc: Refuse x-decimation above 4 for all but 8-bit formats

[Laurent Pinchart]

Hi Jyri,

Thank you for the patch.

On Tuesday 07 Feb 2017 16:41:20 Jyri Sarha wrote:
> Let's disable all scaling that requires horizontal decimation with
> higher factor than 4, until we have better estimates of what we can
> and can not do. However, 1 byte per pixel color format appear to work
> Ok with all decimation factors.
> 
> When decimating horizontally by more that 4 the dss is not able to
> fetch the data in burst mode. When this happens it is hard to tell if
> there enough bandwidth. Despite what theory says this appears to be
> true also for 16-bit color formats.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> b/drivers/gpu/drm/omapdrm/dss/dispc.c index 5554b72..61daef6 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -2506,6 +2506,25 @@ static int dispc_ovl_calc_scaling_44xx(unsigned long
> pclk, unsigned long lclk, return -EINVAL;
>  	}
> 
> +	if (*decim_x > 4 && color_mode_to_bpp(color_mode) > 8) {
> +		/*
> +		  Let's disable all scaling that requires horizontal
> +		  decimation with higher factor than 4, until we have
> +		  better estimates of what we can and can not
> +		  do. However, 1 byte per pixel color format appear to
> +		  work Ok with all decimation factors.
> +
> +		  When decimating horizontally by more that 4 the dss
> +		  is not able to fetch the data in burst mode. When
> +		  this happens it is hard to tell if there enough
> +		  bandwidth. Despite what theory says this appears to
> +		  be true also for 16-bit color formats.
> +		*/
> +		DSSERR("Not enough bandwidth (x-decimation factor %d > 4)",
> +			*decim_x);
> +		return -EINVAL;

This needs to be validated during the atomic check phase to avoid failures at 
commit time that are much harder to handle properly.

> +	}
> +
>  	*core_clk = dispc.feat->calc_core_clk(pclk, in_width, in_height,
>  				out_width, out_height, mem_to_mem);
>  	return 0;

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omapdrm: dispc: Refuse x-decimation above 4 for all but 8-bit formats

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 2254 bytes --]

On 08/02/17 13:25, Laurent Pinchart wrote:
> Hi Jyri,
> 
> Thank you for the patch.
> 
> On Tuesday 07 Feb 2017 16:41:20 Jyri Sarha wrote:
>> Let's disable all scaling that requires horizontal decimation with
>> higher factor than 4, until we have better estimates of what we can
>> and can not do. However, 1 byte per pixel color format appear to work
>> Ok with all decimation factors.
>>
>> When decimating horizontally by more that 4 the dss is not able to
>> fetch the data in burst mode. When this happens it is hard to tell if
>> there enough bandwidth. Despite what theory says this appears to be
>> true also for 16-bit color formats.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/dss/dispc.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
>> b/drivers/gpu/drm/omapdrm/dss/dispc.c index 5554b72..61daef6 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
>> @@ -2506,6 +2506,25 @@ static int dispc_ovl_calc_scaling_44xx(unsigned long
>> pclk, unsigned long lclk, return -EINVAL;
>>  	}
>>
>> +	if (*decim_x > 4 && color_mode_to_bpp(color_mode) > 8) {
>> +		/*
>> +		  Let's disable all scaling that requires horizontal
>> +		  decimation with higher factor than 4, until we have
>> +		  better estimates of what we can and can not
>> +		  do. However, 1 byte per pixel color format appear to
>> +		  work Ok with all decimation factors.
>> +
>> +		  When decimating horizontally by more that 4 the dss
>> +		  is not able to fetch the data in burst mode. When
>> +		  this happens it is hard to tell if there enough
>> +		  bandwidth. Despite what theory says this appears to
>> +		  be true also for 16-bit color formats.
>> +		*/
>> +		DSSERR("Not enough bandwidth (x-decimation factor %d > 4)",
>> +			*decim_x);
>> +		return -EINVAL;
> 
> This needs to be validated during the atomic check phase to avoid failures at 
> commit time that are much harder to handle properly.

I agree, but that requires rewriting half of the dispc driver... This
and the few earlier ones from Jyri are quick fixes to major issues we've
found.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omapdrm: dispc: Refuse x-decimation above 4 for all but 8-bit formats

[Jyri Sarha]

On 02/08/17 13:25, Laurent Pinchart wrote:
> Hi Jyri,
> 
> Thank you for the patch.
> 
> On Tuesday 07 Feb 2017 16:41:20 Jyri Sarha wrote:
>> Let's disable all scaling that requires horizontal decimation with
>> higher factor than 4, until we have better estimates of what we can
>> and can not do. However, 1 byte per pixel color format appear to work
>> Ok with all decimation factors.
>>
>> When decimating horizontally by more that 4 the dss is not able to
>> fetch the data in burst mode. When this happens it is hard to tell if
>> there enough bandwidth. Despite what theory says this appears to be
>> true also for 16-bit color formats.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/dss/dispc.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
>> b/drivers/gpu/drm/omapdrm/dss/dispc.c index 5554b72..61daef6 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
>> @@ -2506,6 +2506,25 @@ static int dispc_ovl_calc_scaling_44xx(unsigned long
>> pclk, unsigned long lclk, return -EINVAL;
>>  	}
>>
>> +	if (*decim_x > 4 && color_mode_to_bpp(color_mode) > 8) {
>> +		/*
>> +		  Let's disable all scaling that requires horizontal
>> +		  decimation with higher factor than 4, until we have
>> +		  better estimates of what we can and can not
>> +		  do. However, 1 byte per pixel color format appear to
>> +		  work Ok with all decimation factors.
>> +
>> +		  When decimating horizontally by more that 4 the dss
>> +		  is not able to fetch the data in burst mode. When
>> +		  this happens it is hard to tell if there enough
>> +		  bandwidth. Despite what theory says this appears to
>> +		  be true also for 16-bit color formats.
>> +		*/
>> +		DSSERR("Not enough bandwidth (x-decimation factor %d > 4)",
>> +			*decim_x);
>> +		return -EINVAL;
> 
> This needs to be validated during the atomic check phase to avoid failures at 
> commit time that are much harder to handle properly.
> 

Sure, but I can not fix everything in one step. This is a first simple
quantitative test that is supposed to tell if scaling can be done
without synclost errors. Next step is to make dispc to not look to HW
for current clock settings, and then finally make all relevant check
callbacks (or some flag in the config callbacks) in dispc.

Anyway, there will be a v2 of this patch that tests color_mode !=
OMAP_DSS_COLOR_NV12 instead of color_mode_to_bpp(color_mode) > 8. The
NV12 appears to be the only color format that works reliably with
current implementation.

Best regards,
Jyri

>> +	}
>> +
>>  	*core_clk = dispc.feat->calc_core_clk(pclk, in_width, in_height,
>>  				out_width, out_height, mem_to_mem);
>>  	return 0;
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omapdrm: dispc: Refuse x-decimation above 4 for all but 8-bit formats

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 2007 bytes --]


On 07/02/17 16:41, Jyri Sarha wrote:
> Let's disable all scaling that requires horizontal decimation with
> higher factor than 4, until we have better estimates of what we can
> and can not do. However, 1 byte per pixel color format appear to work
> Ok with all decimation factors.
> 
> When decimating horizontally by more that 4 the dss is not able to
> fetch the data in burst mode. When this happens it is hard to tell if
> there enough bandwidth. Despite what theory says this appears to be
> true also for 16-bit color formats.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 5554b72..61daef6 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -2506,6 +2506,25 @@ static int dispc_ovl_calc_scaling_44xx(unsigned long pclk, unsigned long lclk,
>  		return -EINVAL;
>  	}
>  
> +	if (*decim_x > 4 && color_mode_to_bpp(color_mode) > 8) {
> +		/*
> +		  Let's disable all scaling that requires horizontal
> +		  decimation with higher factor than 4, until we have
> +		  better estimates of what we can and can not
> +		  do. However, 1 byte per pixel color format appear to
> +		  work Ok with all decimation factors.
> +
> +		  When decimating horizontally by more that 4 the dss
> +		  is not able to fetch the data in burst mode. When
> +		  this happens it is hard to tell if there enough
> +		  bandwidth. Despite what theory says this appears to
> +		  be true also for 16-bit color formats.
> +		*/
> +		DSSERR("Not enough bandwidth (x-decimation factor %d > 4)",
> +			*decim_x);

I think the error message could be improved. A normal user could hit
this when setting up a plane, and I'm quite sure the above doesn't give
any clue to the user what the issue is (too much downscaling).

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omapdrm: dispc: Refuse x-decimation above 4 for all but 8-bit formats

[Laurent Pinchart]

Hi Tomi,

On Wednesday 08 Feb 2017 15:51:08 Tomi Valkeinen wrote:
> On 07/02/17 16:41, Jyri Sarha wrote:
> > Let's disable all scaling that requires horizontal decimation with
> > higher factor than 4, until we have better estimates of what we can
> > and can not do. However, 1 byte per pixel color format appear to work
> > Ok with all decimation factors.
> > 
> > When decimating horizontally by more that 4 the dss is not able to
> > fetch the data in burst mode. When this happens it is hard to tell if
> > there enough bandwidth. Despite what theory says this appears to be
> > true also for 16-bit color formats.
> > 
> > Signed-off-by: Jyri Sarha <jsarha@ti.com>
> > ---
> > 
> >  drivers/gpu/drm/omapdrm/dss/dispc.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> > b/drivers/gpu/drm/omapdrm/dss/dispc.c index 5554b72..61daef6 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> > @@ -2506,6 +2506,25 @@ static int dispc_ovl_calc_scaling_44xx(unsigned
> > long pclk, unsigned long lclk,> 
> >  		return -EINVAL;
> >  	
> >  	}
> > 
> > +	if (*decim_x > 4 && color_mode_to_bpp(color_mode) > 8) {
> > +		/*
> > +		  Let's disable all scaling that requires horizontal
> > +		  decimation with higher factor than 4, until we have
> > +		  better estimates of what we can and can not
> > +		  do. However, 1 byte per pixel color format appear to
> > +		  work Ok with all decimation factors.
> > +
> > +		  When decimating horizontally by more that 4 the dss
> > +		  is not able to fetch the data in burst mode. When
> > +		  this happens it is hard to tell if there enough
> > +		  bandwidth. Despite what theory says this appears to
> > +		  be true also for 16-bit color formats.
> > +		*/
> > +		DSSERR("Not enough bandwidth (x-decimation factor %d > 4)",
> > +			*decim_x);
> 
> I think the error message could be improved. A normal user could hit
> this when setting up a plane, and I'm quite sure the above doesn't give
> any clue to the user what the issue is (too much downscaling).

Shouldn't the message be turned into a debug message ? Otherwise you'll give a 
way for users to flood the kernel log, which is never good.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel