[PATCH v3 06/18] tegra: fdt: Add LCD definitions for Tegra

[Simon Glass]

Hi Stephen,

On Thu, Sep 27, 2012 at 4:21 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 09/27/2012 02:27 PM, Simon Glass wrote:
>> On Thu, Sep 27, 2012 at 8:49 AM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>> On 09/27/2012 07:58 AM, Simon Glass wrote:
> ...
>>>> Really this is just a way of getting U-Boot and Linux to agree on the
>>>> address, by having U-Boot know the address that the kernel will happen
>>>> to use. It isn't very robust, but we have found it useful as a means
>>>> of avoiding problems in this area.
>>>
>>> Right, but again, why can't the display driver simply read the address
>>> out of the register; why is there a need to duplicate the data?
>>>
>>> I guess one possibility is that the register only gives the base address
>>> and not the size/mode of the allocated surface, but I don't recall if
>>> this proposed binding did that either?
>>
>> So here is my explanation:
>>
>> 1. U-Boot would normally put the display right near the top of memory.
>> Instead, it figures out where the kernel will put it (somehow this
>> seems to often be at a fixed address) and uses that address.
>>
>> 2. That means that U-Boot will now have the display exactly where the
>> kernel wants it.
>
> Oh, the DT property is telling U-Boot where to put the surface so that
> when the kernel determines where to put it, it'll already be there?
>
> That seems completely backwards. It's also extremely fragile; what if
> the kernel suddenly starts allocating at some other random address,
> which stops matching what U-Boot expects?

The screen will flicker, or may fill up with junk as the kernel boots.
We did have that happen once when the driver changed.

>
> Far better would be for U-Boot to put the surface wherever it wants,
> then manipulate the DT that's passed to the kernel to:
>
> a) Add a /memreserve/ so that memory doesn't get re-used for anything
> else during boot.
>
> b) Add a property to the display node indicating which memreserve
> represents the frame-buffer location.

Yes, I believe the Tegra video driver we used (and I take your point
that it doesn't exist upstream so there is no point in discussing it
:-) always had to use the same frame buffer address. The memory was
statically allocated. Certainly this could be encoded in the fdt.

>
> When the kernel boots, it can:
>
> * Copy the content from the U-Boot-allocated buffer to whatever display
> buffer the kernel allocated before writing the new address to the HW.
>
> * Undo the memory reservation triggered by the /memreserve/ to allow the
> memory to be re-used.
>
>> So what do you think? Should we remove this entirely, or is it a useful hack?
>
> Yes, I think in its current form, it isn't useful.

Thanks for the thorough response. Let's remove it, particularly as
there isn't even a kernel driver upstream yet.

I will do that before sending out the LCD v4 series. Thanks for your
timely comments.

Regards,
Simon