Re: [PATCH 1/2] OMAPDSS: DISPC: Update Fir Coefficients

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 1746 bytes --]

On Wed, 2011-12-14 at 10:21 +0530, Chandrabhanu Mahapatra wrote:

> +const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps)
> +{
> +	int i;
> +	static const struct {
> +		int Mmin;
> +		int Mmax;
> +		const struct dispc_coef *coef_3;
> +		const struct dispc_coef *coef_5;
> +	} coefs[] = {
> +		{ 26, 32, coef3_M32, coef5_M32 },
> +		{ 22, 26, coef3_M26, coef5_M26 },
> +		{ 19, 22, coef3_M22, coef5_M22 },
> +		{ 16, 19, coef3_M19, coef5_M19 },
> +		{ 14, 16, coef3_M16, coef5_M16 },
> +		{ 13, 14, coef3_M14, coef5_M14 },
> +		{ 12, 13, coef3_M13, coef5_M13 },
> +		{ 11, 12, coef3_M12, coef5_M12 },
> +		{ 10, 11, coef3_M11, coef5_M11 },
> +		{  9, 10, coef3_M10, coef5_M10 },
> +		{  8,  9,  coef3_M9,  coef5_M9 },
> +		{  3,  8,  coef3_M8,  coef5_M8 },
> +		/*
> +		 * When upscaling more than two times, blockiness and outlines
> +		 * around the image are observed when M8 tables are used. M11,
> +		 * M16 and M19 tables are used to prevent this.
> +		 */
> +		{  2,  3, coef3_M11, coef5_M11 },
> +		{  1,  2, coef3_M16, coef5_M16 },
> +	};
> +
> +	inc /= 128;
> +	for (i = 0; i < ARRAY_LEN(coefs); ++i)
> +		if (inc > coefs[i].Mmin && inc <= coefs[i].Mmax)
> +			return five_taps ? coefs[i].coef_5 : coefs[i].coef_3;
> +	if (inc == 1)
> +		return five_taps ? coef3_M19 : coef5_M19;
> +	return NULL;
> +}

Why don't you handle the inc == 1 case the same as others? Just have an
entry in the table for Mmin=0, Mmax = 1.

Also, I think it's a bit confusing that Mmin is exclusive and Mmax is
inclusive in the comparison. It makes the table a bit hard to read, when
looking at which entry is used for which inc. I'd recommend using
inclusive comparison for both.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]