[U-Boot] [RESEND] video: bcm2835: add support for reading from the video-mode environment variable

[Stephen Warren]

On 01/18/2017 07:15 PM, Brian Masney wrote:
> On Fri, Jan 13, 2017 at 10:21:15AM -0700, Stephen Warren wrote:
>> On 01/13/2017 04:48 AM, Brian Masney wrote:
>>> On Thu, Jan 12, 2017 at 11:47:48AM -0700, Stephen Warren wrote:
>>>> On 01/12/2017 11:32 AM, Brian Masney wrote:
>>>>> On Thu, Jan 12, 2017 at 11:02:14AM -0700, Stephen Warren wrote:
>>>>>> On 01/12/2017 01:57 AM, Brian Masney wrote:
>>>>>>> The bcm2835 driver polls the monitor and selects the highest resolution
>>>>>>> that is available. This patch allows optionally setting the video-mode
>>>>>>> environment variable so that a different video resolution can be used.
>>>>>>> If the environment variable is not specified, then it will fall back to
>>>>>>> using the old behavior of using the maximum allowable resolution.
>>>>>>>
>>>>>>> This patch is needed to fix an issue booting an upstream Linux kernel
>>>>>>> on a Raspberry Pi 2 with a Pi Top screen. Previously, the bcm2835 would
>>>>>>> select the 1366x768 resolution (which is a supported resolution),
>>>>>>> however the screen would be unreadable. (See
>>>>>>> https://www.flickr.com/photos/masneyb/30942037416/ for picture). Using
>>>>>>> this patch, the resolution 1024x768 can be selected and is readable on
>>>>>>> the screen.
>>>>>>
>>>>>> Doesn't this mean that the RPi firmware is reporting the wrong resolution?
>>>>>> If so, isn't the correct fix to get an updated firmware that reports the
>>>>>> correct resolution, rather than patching each piece of SW to ignore the
>>>>>> FW-reported resolution? Or, if this is caused by incorrect EDID in the Pi
>>>>>> Top, then fix the EDID EEPROM on that.
>>>>>>
>>>>>> Perhaps there are other use-cases for using a non-default resolution, but to
>>>>>> support that, you'd need to make a call into the FW to request and configure
>>>>>> that non-default resolution, not just ignore what resolution the FW
>>>>>> programmed.
>>>>>
>>>>> Hi Stephen,
>>>>>    The Pi Top screen works correctly with the 1366x768 resolution when
>>>>> booting the 4.4 kernel provided by the Raspberry Pi foundation in
>>>>> stock Raspbian (no u-boot). (There are no outside provided drivers from
>>>>> Pi Top.) When booting with u-boot, I can't use the 1366x768 resolution,
>>>>> even when setting the resolution manually using my patch. When auto
>>>>> detection is in place, u-boot correctly detects the 1366x768 resolution
>>>>> according to debugging messages that I see on the serial console.
>>>>> 1024x768 is the highest resolution that I can get a working display with
>>>>> the Pi Top and u-boot. I also tried changing the display depth.
>>>>>
>>>>>    I tried booting u-boot using the latest Raspberry Pi firmware and the
>>>>> older firmware provided with the Raspbian 4.4 kernel. In both cases, the
>>>>> screen correctly displays the rainbow square at the top left when the
>>>>> GPU is booting, but then the screen becomes garbled when it gets to
>>>>> u-boot and the bcm2835 code sets the resolution.
>>>>>
>>>>>    I tried going through the Pi Top vendor for help but didn't get very
>>>>> far with them. I'm open to other suggestions to try.
>>>>
>>>> Is the bad image that you get static/fixed, or does it move about randomly?
>>>>
>>>> If it's static/fixed, I wonder if the issue is with the memory pitch
>>>> calculation. What value does bcm2835_pitch have without your patch? (and
>>>> with your patch, I think it's uninitialized?).
>>>>
>>>> I wonder if you round the value of bcm2835_pitch up to a multiple of 256 (or
>>>> something like that), then perhaps the issue may be solved? If you change
>>>> the pitch value, and you notice the angle of the diagonal edges in the image
>>>> change, the issue is almost certainly related to this pitch value.
>>>>
>>>> I can't recall how the mainline kernel driver works now. If it also uses the
>>>> property mailbox to configure the display, and you're just using the dumb
>>>> simplefb driver, perhaps you can compare the memory layout parameters the
>>>> kernel uses when it's working to what U-Boot uses when it's not working.
>>>
>>> The image is fixed. I can see when the Linux Kernel boots and the
>>> console messages scroll across the screen.
>>>
>>> Without my patch, u-boot detects the screen resolution as 1366x768 with
>>> a pitch of 5504. With my patch, 1024x768 uses a pitch of 2048.
>>>
>>> I reverted my u-boot patch and tried hard coding the pitch to the values
>>> 5376, 5632, 2048, 4096, and 6144 with no success. Once I read what the
>>> pitch value does, I also tried 5464 (1366 x 4 (for 32bpp)) and 2732.
>>
>> Is this related?
>>
>>> https://www.riscosopen.org/forum/forums/4/topics/6400?posts_per_page=25&posts_per_page_change=Change
>> (See "I think I've just fixed that")
>>
>>> https://www.riscosopen.org/viewer/revisions/logs?ident=1471703957-443671.html
>> (See the diff in the BCMVideo file)
>>
>> Another thing to try might be to remove the SET_* operations from U-Boot's
>> property mailbox calls (I think the FW always initializes video out anyway,
>> so U-Boot doesn't need to set anything up), and simply invoke the relevant
>> GET_* operations to query where the buffer is and its size. IIRC I didn't do
>> that because you can only query certain information as a side-effect of
>> asking the FW to allocate a frame-buffer though (i.e. the info comes back in
>> a SET_xxx/ALLOCATE_xxx response message, but there's no GET_xxx to query the
>> data without modifying the configuration or re-allocating anything).
>
> I attached a patch (uboot-bcm2835-release-fb.patch) that releases the frame
> buffer before the other mailbox properties are set. This did not fix my display
> problem.

Hmm. That's a pity.

> I also tried removing all of the SET_ operations
> (uboot-bcm2835-remote-mbox-sets.patch). The screen stays on the rainbow
> screen when the GPU is initialized. I can see through the serial console
> that the pi boots normally.
>
> Applying both patches gives a blank screen so it appears that releasing
> the frame buffer works.
>
> In the BCMVideo diff that you referenced, there are two rows between
> ARM2VC_Tag_FBRelease and ARM2VC_Tag_End with zeros:
>
> +tagrel  DCD     tagrellen
> +        DCD     0

Those two should be this in U-Boot:

/* All message buffers must start with this header */
struct bcm2835_mbox_hdr {
         u32 buf_size;
         u32 code;
};

> +        DCD     ARM2VC_Tag_FBRelease
> +        DCD     0
> +        DCD     0

Those three should be this in U-Boot:

/*
  * A message buffer contains a list of tags. Each tag must also start with
  * a standardized header.
  */
struct bcm2835_mbox_tag_hdr {
         u32 tag;
         u32 val_buf_size;
         u32 val_len;
};

> +        DCD     ARM2VC_Tag_End
> +tagrellen *     . - tagrel
>
> Are these parameters? I don't see places to put those in
> bcm2835_mbox_tag_release_buffer.

...
> I also forgot to mention in my last email that changing the value of
> bcm2835_pitch does not cause the output to move.

Oh. That's odd. I'd suggest asking about this on the Raspberry Pi 
forums, or perhaps on github as a bug/question in the Raspberry Pi 
firmware repository. There's nothing U-Boot-specific about all this; 
U-Boot is just sending in standard messages that the RPi firmware 
understands.