mbox series

[v3,0/6] Generic USB Display driver

Message ID 20200529175643.46094-1-noralf@tronnes.org (mailing list archive)
Headers show
Series Generic USB Display driver | expand

Message

Noralf Trønnes May 29, 2020, 5:56 p.m. UTC
Hi,

A while back I had the idea to turn a Raspberry Pi Zero into a $5
USB to HDMI/SDTV/DSI/DPI display adapter.

This series adds a USB host driver and a device/gadget driver to achieve
that.

The reason for calling it 'Generic' is so anyone can make a USB
display/adapter against this driver, all that's needed is to add a USB
vid:pid. I have done a microcontroller implementation hack just to see
how that would work out[1] (which unconvered a couple of bugs in the
host driver).

The contents of the previous cover letter has been moved to the wiki[2]
since it was getting rather long.

I've made an image[3] with the gadget side set up for the Raspberry Pi
for easy testing. Patch 4 is the only one needed for the host side.

Merge plan
I'm hoping to apply the remaining drm_client patches in time for 5.9.
With that in place it's much easier to apply the patch for the USB
subsystem the following merge window (5.10). Doing both in the same
cycle is possible ofc, but due to the high rate of change in DRM this
_can_ turn out to be tricky. There's no hurry to get the gadget side
merged since I will provide images for the Raspberry Pi. The host driver
I hope to apply in time for 5.9.
Reviews and testing are very much welcome!

Changes since version 2:
- Use donated Openmoko USB pid: 1d50:614d
- Use direct compression from framebuffer when pitch matches, not only
  on full frames, so split updates can benefit
- Use __le16 in struct gud_drm_req_get_connector_status
- Set edid property when the device only provides edid
- Clear compression fields in struct gud_drm_req_set_buffer
- Fix protocol version negotiation
- Remove mode->vrefresh, it's calculated
- drm_client_init_from_id(): remove locking
- Applied reviewed patches, thanks Sam.

Dependency:
- backlight: Add backlight_device_get_by_name()[4]

Noralf.

[1] https://github.com/notro/gud/tree/master/STM32F769I-DISCO
[2] https://github.com/notro/gud/wiki
[3] https://github.com/notro/gud/wiki/rpi-image
[4] https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git/commit/?h=for-backlight-next&id=479da1f538a2f3547e15f9d5922c611b69ec2fbc


Noralf Trønnes (6):
  drm/client: Add drm_client_init_from_id()
  drm/client: Add drm_client_modeset_disable()
  drm/client: Add a way to set modeset, properties and rotation
  drm: Add Generic USB Display driver
  drm/gud: Add functionality for the USB gadget side
  usb: gadget: function: Add Generic USB Display support

 .../ABI/testing/configfs-usb-gadget-gud_drm   |   10 +
 MAINTAINERS                                   |   10 +
 drivers/gpu/drm/Kconfig                       |    2 +
 drivers/gpu/drm/Makefile                      |    1 +
 drivers/gpu/drm/drm_client.c                  |   44 +-
 drivers/gpu/drm/drm_client_modeset.c          |  157 +++
 drivers/gpu/drm/gud/Kconfig                   |   20 +
 drivers/gpu/drm/gud/Makefile                  |    5 +
 drivers/gpu/drm/gud/gud_drm_connector.c       |  726 ++++++++++
 drivers/gpu/drm/gud/gud_drm_drv.c             |  648 +++++++++
 drivers/gpu/drm/gud/gud_drm_gadget.c          | 1167 +++++++++++++++++
 drivers/gpu/drm/gud/gud_drm_internal.h        |   65 +
 drivers/gpu/drm/gud/gud_drm_pipe.c            |  426 ++++++
 drivers/usb/gadget/Kconfig                    |   12 +
 drivers/usb/gadget/function/Makefile          |    2 +
 drivers/usb/gadget/function/f_gud_drm.c       |  678 ++++++++++
 include/drm/drm_client.h                      |   43 +-
 include/drm/gud_drm.h                         |  374 ++++++
 18 files changed, 4387 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-gud_drm
 create mode 100644 drivers/gpu/drm/gud/Kconfig
 create mode 100644 drivers/gpu/drm/gud/Makefile
 create mode 100644 drivers/gpu/drm/gud/gud_drm_connector.c
 create mode 100644 drivers/gpu/drm/gud/gud_drm_drv.c
 create mode 100644 drivers/gpu/drm/gud/gud_drm_gadget.c
 create mode 100644 drivers/gpu/drm/gud/gud_drm_internal.h
 create mode 100644 drivers/gpu/drm/gud/gud_drm_pipe.c
 create mode 100644 drivers/usb/gadget/function/f_gud_drm.c
 create mode 100644 include/drm/gud_drm.h

Comments

Lubomir Rintel July 9, 2020, 4:32 p.m. UTC | #1
Hello,

On 29 May 2020 Noralf Trønnes wrote:
...
> This series adds a USB host driver and a device/gadget driver to achieve
> that.
> 
> The reason for calling it 'Generic' is so anyone can make a USB
> display/adapter against this driver, all that's needed is to add a USB
> vid:pid. I have done a microcontroller implementation hack just to see
> how that would work out[1] (which unconvered a couple of bugs in the
> host driver).
...

This is actually very cool; finally a good way to drive the cheapo
SPI/I2C displays from computers whose only means of expansion is USB
with a little help from a microcontroller. I've actually had some
success doing just that [1].

[1] https://assets.octodon.social/media_attachments/files/009/983/960/original/64ad8ea46c1b06c5.jpg

I suppose you can add:

Tested-by: Lubomir Rintel <lkundrak@v3.sk>

I've had to jump through some hoops though.

My OLED display is a 128x64 SSD1306 [1] driven from the SPI bus. The frame
buffer SRAM is normally scanned out in stripes of 8 vertical pixels called
"pages". When the display is turned on its side, with "vertical
addressing mode" and "segment remapping" enabled and bytes being sent LSB
first, it appears linear -- it's easy to repaint the whole display from
what is now the top left corner to the bottom right. This is very
convenient for painting pixels as they come, without bufferring them or
doing any conversions (assuming that memory and cpu cycles are at
premium on MCUs).

[1] https://cdn-shop.adafruit.com/datasheets/SSD1306.pdf

There doesn't seem a comfortable way to do partial redraws though. Would
you find it objectionable if the device could indicate that needs full
frames instead of just the damaged areas? Perhaps then the driver
wouldn't even need to bother issuing GUD_DRM_USB_REQ_SET_BUFFER to
displays dumb enough to be incapable of partial redraws and decompression.

My work-in-progress code that works on STM32F103 (e.g. "Blue Pill"
boards), or GD32VF103 (RISC-V "Polos Alef") is at [2]. The partial redraws
that don't start from column zero or are not "page aligned" don't work
correctly for the time being; X11 doesn't seem to care.

[2] https://github.com/hackerspace/libopencm3-gf32v-examples/tree/lr/gd32v/examples/gd32v/f103/polos-alef/usb-display

Thank you!
Lubo
Noralf Trønnes July 14, 2020, 1:33 p.m. UTC | #2
Den 09.07.2020 18.32, skrev Lubomir Rintel:
> Hello,
> 
> On 29 May 2020 Noralf Trønnes wrote:
> ...
>> This series adds a USB host driver and a device/gadget driver to achieve
>> that.
>>
>> The reason for calling it 'Generic' is so anyone can make a USB
>> display/adapter against this driver, all that's needed is to add a USB
>> vid:pid. I have done a microcontroller implementation hack just to see
>> how that would work out[1] (which unconvered a couple of bugs in the
>> host driver).
> ...
> 
> This is actually very cool; finally a good way to drive the cheapo
> SPI/I2C displays from computers whose only means of expansion is USB
> with a little help from a microcontroller. I've actually had some
> success doing just that [1].

Nice to see a monochrome implementation, I actually planned to remove
the R1 format from v3 since the gadget driver doesn't implement it.
Having unused (and poorly tested) code isn't that great, but I forgot.
It turns out it was a good thing that I forgot that :-)

> 
> [1] https://assets.octodon.social/media_attachments/files/009/983/960/original/64ad8ea46c1b06c5.jpg
> 
> I suppose you can add:
> 
> Tested-by: Lubomir Rintel <lkundrak@v3.sk>
> 
> I've had to jump through some hoops though.
> 
> My OLED display is a 128x64 SSD1306 [1] driven from the SPI bus. The frame
> buffer SRAM is normally scanned out in stripes of 8 vertical pixels called
> "pages". When the display is turned on its side, with "vertical
> addressing mode" and "segment remapping" enabled and bytes being sent LSB
> first, it appears linear -- it's easy to repaint the whole display from
> what is now the top left corner to the bottom right. This is very
> convenient for painting pixels as they come, without bufferring them or
> doing any conversions (assuming that memory and cpu cycles are at
> premium on MCUs).
> 
> [1] https://cdn-shop.adafruit.com/datasheets/SSD1306.pdf
> 
> There doesn't seem a comfortable way to do partial redraws though. Would
> you find it objectionable if the device could indicate that needs full
> frames instead of just the damaged areas? Perhaps then the driver
> wouldn't even need to bother issuing GUD_DRM_USB_REQ_SET_BUFFER to
> displays dumb enough to be incapable of partial redraws and decompression.
> 

Yeah I figured always having full updates might benefit/simplify
monochrome devices, but I'd wait for an actual device needing it before
adding it. Now that you need it, I'll add a flag for it in the next
version.

I would like to keep the SET_BUFFER request since it will serve as a
syncing point between the host and the device. I'm no USB expert but I
assume that a bulk transfer can fail halfway through and result in the
next update starting where the previous one failed and thus writing
beyond the end of the display buffer.

Noralf.

> My work-in-progress code that works on STM32F103 (e.g. "Blue Pill"
> boards), or GD32VF103 (RISC-V "Polos Alef") is at [2]. The partial redraws
> that don't start from column zero or are not "page aligned" don't work
> correctly for the time being; X11 doesn't seem to care.
> 
> [2] https://github.com/hackerspace/libopencm3-gf32v-examples/tree/lr/gd32v/examples/gd32v/f103/polos-alef/usb-display
> 
> Thank you!
> Lubo
>
Peter Stuge July 14, 2020, 5:40 p.m. UTC | #3
Hi Noralf,

Noralf Trønnes wrote:
> I would like to keep the SET_BUFFER request since it will serve as a
> syncing point between the host and the device. I'm no USB expert but I
> assume that a bulk transfer can fail halfway through and result in the
> next update starting where the previous one failed and thus writing
> beyond the end of the display buffer.

Transfers either succeed completely (possibly after many retries),
time out (after zero or more transfered bytes) or fail catastrophically
(e.g. from device disconnect).

In all cases, the driver on the host knows/has available how many bytes
were successfully transfered.


//Peter
Noralf Trønnes July 14, 2020, 7:03 p.m. UTC | #4
Den 14.07.2020 19.40, skrev Peter Stuge:
> Hi Noralf,
> 
> Noralf Trønnes wrote:
>> I would like to keep the SET_BUFFER request since it will serve as a
>> syncing point between the host and the device. I'm no USB expert but I
>> assume that a bulk transfer can fail halfway through and result in the
>> next update starting where the previous one failed and thus writing
>> beyond the end of the display buffer.
> 
> Transfers either succeed completely (possibly after many retries),
> time out (after zero or more transfered bytes) or fail catastrophically
> (e.g. from device disconnect).
> 
> In all cases, the driver on the host knows/has available how many bytes
> were successfully transfered.
> 

I was thinking about the device, that it could get out of sync. Let's
say the host sends a 1k framebuffer and half of it gets transferred and
the rest fails for some reason. Lubomir's MCU implementation has an
endpoint max size of 64 bytes and a callback is called for each packet.
If the 1k transfer fails at some point, will the device be able to
detect this and know that the next time the rx callback is called that
this is the start of a new framebuffer update?

Noralf.
Peter Stuge July 14, 2020, 7:38 p.m. UTC | #5
Noralf Trønnes wrote:
> > In all cases, the driver on the host knows/has available how many bytes
> > were successfully transfered.
> 
> I was thinking about the device, that it could get out of sync. Let's
> say the host sends a 1k framebuffer and half of it gets transferred and
> the rest fails for some reason. Lubomir's MCU implementation has an
> endpoint max size of 64 bytes and a callback is called for each packet.
> If the 1k transfer fails at some point, will the device be able to
> detect this and know that the next time the rx callback is called that
> this is the start of a new framebuffer update?

Ah! No, a device can not detect that the host intended to send more (bulk)
packets but e.g. timed out.

I can't immediately think of other reasons for a larger transfer to fail,
which still allow communication to continue.

When the host recognizes a timeout with partial data transfer it could
simply send the remaining data in a new transfer.


While the device can not detect host intent, the protocol could allow
devices to specify requirements, e.g. that the host always sends full frames.

In any case, please avoid making a control request mandatory for frame
transfer.

Because control requests are scheduled differently onto the wire and
because they consist of more packets than bulk data, a control request
will interrupt a bulk data stream and likely introduce unneccessary latency.

If synchronization is always required then I'd suggest to place it inline
with frame data, e.g. in the first packet byte.

If synchronization is only required in rare cases then a control transfer
is probably the better choice, to not waste any inline bytes.

But the optimum would be that the device can describe its needs to the host
and the host driver ensures that the device always receives the data it needs.

Do you think this is possible?


Kind regards

//Peter
Lubomir Rintel July 15, 2020, 7:30 a.m. UTC | #6
On Tue, Jul 14, 2020 at 09:03:14PM +0200, Noralf Trønnes wrote:
> 
> 
> Den 14.07.2020 19.40, skrev Peter Stuge:
> > Hi Noralf,
> > 
> > Noralf Trønnes wrote:
> >> I would like to keep the SET_BUFFER request since it will serve as a
> >> syncing point between the host and the device. I'm no USB expert but I
> >> assume that a bulk transfer can fail halfway through and result in the
> >> next update starting where the previous one failed and thus writing
> >> beyond the end of the display buffer.
> > 
> > Transfers either succeed completely (possibly after many retries),
> > time out (after zero or more transfered bytes) or fail catastrophically
> > (e.g. from device disconnect).
> > 
> > In all cases, the driver on the host knows/has available how many bytes
> > were successfully transfered.
> > 
> 
> I was thinking about the device, that it could get out of sync. Let's
> say the host sends a 1k framebuffer and half of it gets transferred and
> the rest fails for some reason. Lubomir's MCU implementation has an
> endpoint max size of 64 bytes and a callback is called for each packet.

Note that 64 bytes was chosen totally arbitrarily, without any thought.
Perhaps the full frame of 1024 bytes would work just fine. I'm not
familiar with USB at all.

> If the 1k transfer fails at some point, will the device be able to
> detect this and know that the next time the rx callback is called that
> this is the start of a new framebuffer update?
> 
> Noralf.

Lubo
Noralf Trønnes July 16, 2020, 5:43 p.m. UTC | #7
Den 14.07.2020 21.38, skrev Peter Stuge:
> Noralf Trønnes wrote:
>>> In all cases, the driver on the host knows/has available how many bytes
>>> were successfully transfered.
>>
>> I was thinking about the device, that it could get out of sync. Let's
>> say the host sends a 1k framebuffer and half of it gets transferred and
>> the rest fails for some reason. Lubomir's MCU implementation has an
>> endpoint max size of 64 bytes and a callback is called for each packet.
>> If the 1k transfer fails at some point, will the device be able to
>> detect this and know that the next time the rx callback is called that
>> this is the start of a new framebuffer update?
> 
> Ah! No, a device can not detect that the host intended to send more (bulk)
> packets but e.g. timed out.
> 
> I can't immediately think of other reasons for a larger transfer to fail,
> which still allow communication to continue.
> 
> When the host recognizes a timeout with partial data transfer it could
> simply send the remaining data in a new transfer.
> 
> 
> While the device can not detect host intent, the protocol could allow
> devices to specify requirements, e.g. that the host always sends full frames.
> 
> In any case, please avoid making a control request mandatory for frame
> transfer.
> 
> Because control requests are scheduled differently onto the wire and
> because they consist of more packets than bulk data, a control request
> will interrupt a bulk data stream and likely introduce unneccessary latency.
> 
> If synchronization is always required then I'd suggest to place it inline
> with frame data, e.g. in the first packet byte.
> 
> If synchronization is only required in rare cases then a control transfer
> is probably the better choice, to not waste any inline bytes.
> 
> But the optimum would be that the device can describe its needs to the host
> and the host driver ensures that the device always receives the data it needs.
> 
> Do you think this is possible?
> 

Looking at the host driver I see that I need to fix it so that it
requeues the update if it fails (on SET_BUFFER or bulk out). Currently
it just goes back to sleep waiting for userspace to announce a new change.

I would like to avoid having a special case for retrying the failing
part of a bulk transfer for devices that only want full updates, I would
prefer to use the common error path of retrying the whole update.

This is my suggestion for the new flag:

/*
 * Always send the entire framebuffer when flushing changes.
 * The GUD_DRM_USB_REQ_SET_BUFFER request will not be sent before each
bulk transfer,
 * it will only be sent if the previous bulk transfer had failed. This
is done to
 * inform the device that the previous update failed and that a new one
is started.
 *
 * This flag can not be used in combination with compression.
 */
#define GUD_DRM_DISPLAY_FLAG_FULL_UPDATE	BIT(1)


Noralf.