linux-kernel - Re: [PATCH 6/8] drm/i2c: tda998x: fix sync generation and calculation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <520B90E1.5000803@gmail.com>
Date:	Wed, 14 Aug 2013 16:14:57 +0200
From:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
CC:	David Airlie <airlied@...ux.ie>,
	Darren Etheridge <detheridge@...com>,
	Rob Clark <robdclark@...il.com>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/8] drm/i2c: tda998x: fix sync generation and calculation

On 08/14/13 14:41, Russell King - ARM Linux wrote:
> On Tue, Aug 06, 2013 at 12:20:16AM +0200, Sebastian Hesselbarth wrote:
>> +	de_pix_s     = mode->htotal - mode->hdisplay;
>> +	de_pix_e     = de_pix_s + mode->hdisplay;
>> +	hs_pix_s     = mode->hsync_start - mode->hdisplay;
>> +	hs_pix_e     = hs_pix_s + mode->hsync_end - mode->hsync_start;
>
> I still think the above is over-complicating the calculation and making it
> less readable.

Yeah, you also didn't like it the last time. I must admit, I have
left it that way because I did all the near-end/far-end scope checks
on the calculation above and I wasn't that eager to touch them again.

But I agree and will revert the lines in question and update the
others accordingly.

> The values in 'mode' represent this timing representation:
> 0=hds                                    hde    hss  hse   ht
> |-----------------------------------------|----->|--->|---->|
>
> What we want to do is to rotate that around to this:
> 0     hss  hse   hds                                       ht=hde
> |----->|--->|---->|-----------------------------------------|
>
>  From that, you can see quite clearly that the end of the displayed line
> is now at htotal, and the start of the displayed line (hds) is hdisplay
> clocks before that.  So:
>
> 	de_pix_e = mode->htotal;
> 	de_pix_s = de_pix_e - mode->hdisplay;
>
> Everything else (hss, hse) has moved to the left by hdisplay clocks, so:
>
> 	hs_pix_s = mode->hsync_start - mode->hdisplay;
> 	hs_pix_e = mode->hsync_end - mode->hdisplay;
>
> That's what you get if you simplify your calculations as well.  If we then
> go back and look at the original code:
>
> -       hs_start   = mode->hsync_start - mode->hdisplay;
> -       hs_end     = mode->hsync_end - mode->hdisplay;
> -       de_start   = mode->htotal - mode->hdisplay;
> -       de_end     = mode->htotal;
>
> it's what was originally there, so I don't see any point in touching that
> calculation.
>
> We also have:
>
> -       ref_pix = 3 + hs_start;
> +       ref_pix      = 3 + mode->hsync_start - mode->hdisplay;
>
> which we can see is also the same calculation in essence.  I don't think
> the change helps readability.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/