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