Hi Luc,

On 08/13/2014 07:55 AM, Luc Verhaegen wrote:
> This is a second spin of a patchset which adds a hdmi driver for u-boots
> cfbconsole.
> 
> The changes are, compared to the previous version:
> * split up
> * added clock claiming code (judge for yourself how nasty this code is today
> and how much nicer it could've been if clocks were handled better in dt).
> * removed a leftover variable from the sunxi_display struct.

Thanks for your continued work on this.

I've been running some tests on this, and also done an imcomplete review of the
code while working on it.

Here are a list of various things I noticed / various review remarks.

I've created patches for some of these as part of my testing, you can find
those here:

https://github.com/jwrdegoede/u-boot-sunxi/commits/next

Feel free to squash them into 1/4 in v3.

Note I'm going on vacation starting tomorrow, I'll be back on Mon Aug 15.

So first some review remarks:

1) This seems to be based on the u-boot-sunxi repository. Most sunxi code
is upstream now. Can you please base your next version on the upstream u-boot
repository?  Note that atm upstream does not support booting linux-suni-3.4 
kernels,
this is mostly a matter of adding machine-ids. I did not do that so far as I 
thought
that Allwinner had just picked random numbers, but they have actually properly
registered the board-ids. So this can go upstream. With this fixed there still 
is
an incompatibility between the sunxi 3.4 kernels and upstream u-boot for A20
devices which I need to track down. I'll look into this after my vacation.

2) Nitpick, sorry, you seem to be using caps in hex notations, e.g. 0xFFFFFFFF
where as almost all other uboot cases uses lower case, please switch to 
lowercase.
Also while nitpicking can you please run the patches through checkpatch ? 
Thanks.

3) I really don't like the enable video by default patch (4/4). Luckily upstream
u-boot has Kcnfig now, so I've simply turned this into a Kconfig option which
defaults to Y. See:

https://github.com/jwrdegoede/u-boot-sunxi/commit/756ca900013c52a7a51ac4fe13f78e709b6749c1

So lets go over the other patches I've done one by one:

1) I happened to grab an A10s / sun5i board to test this, the
clock-output-names check fails there, I've a patch fixing this.

2) My monitor would not sync at the default 1024x768, so I added
a 1920x1080 mode, feel free to drop this patch is is just for convenient
testing at 1920x1080

3) I was not really charmed about reading back the lcd0ch1 clk reg to figure out
the divider and whether or not to use clock doubling. So I've done a small patch
to instead pass the values as initially calculated around.

4) Working on 3). made me realize that the clk setup code tries to use m = 16 
which
won't work.

5) I noticed that some of the "magic" values for the hdmi tx driver regs were
different from those found in the sunxi -3.4 kernel, changing these to the
3.4 kernels fixes my monitor not syncing on 1024x768.

Question why were these changed ?

6) And last add a small tweak also from the 3.4 kernel to fix the hdmi fifo
sometimes underrunning leading to small speckles on the screen.

That about wraps it up. I'll catch up with you after my vacation.

Regards,

Hans

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to