Re: [RFC PATCH 0/6] tiny: Add driver for gooddisplay epaper panels

["Jan Sebastian Götte" <linux@jaseg.net>]

Hi Daniel,

thanks for your comments!

On 7/30/19 11:26 PM, Daniel Vetter wrote:
>> I have a couple of questions regarding this driver, thus the RFC:
>> (1) By default, when loading the module a vt console binds to the fb. I think this is useful, but the cursor blink of the vt leads to an eternal refresh loop. Refresh on these displays is *extremely* slow (1 frame per 15s), so ideally I'd want the cursor to not blink. Is there some nice way to tell the vt/console driver to do this by default?
> 
> Hm maybe there is something, but this is the first epaper driver, so I
> guess even if it's exists already in fbcon, you'd need to wire through
> a flag from drm to drm fbdev emulation to fbdev core to fbcon that
> cursors should better not blink.

Thanks. I'll look into that.

>> (2) Is ioctl the correct interface for the driving waveform/refresh stuff?
> 
> For kms, no. In general kms is supposed to be standardized and
> generic, and uses properties. I think Emil already comment a bit with
> pointers what you should look into instead for each part. For partial
> updates we have the damage rectangle stuff now, but no idea whether
> that's good enough for what you need.

I suspect not. I looked into these properties, but as far as I understood they're tied to a mode, and modes are kind of static? If you want to display an animation on the epaper you might have to change these on a frame-per-frame basis. Can that be done with kms properties?

>> (3) I read that any drm driver has to be committed along with a libdrm implementation. I think the most likely interface for anyone to interact with this driver would be the fb dev. Do I have to make some userspace implementation as well anyway? If so, where would that go: libdrm or some sort of new libepaper?
> 
> Even more strict: we require full open source userspace, per
> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
> 
> But since kms is standardized you should be able to create a driver
> for it with no new changes in userspace at all.

phew!

>> (4) The driver accepts both XRGB8888 and RGB565 (for compatibility with small LCDs). The driver's current approach to calculate a b/w/r "ternary" image from this data is to just take the MSBs of each color component, then make anything red (R>127, G,B<=127) red, anything black (R,G,B each <=127) black and anything else white. This is since the display's default state is white, and a pixel can turn either red or black from that. Note that it's the actual pixel changing color, i.e. there is no black and red sub-pixels. If you try to drive black and red at the same time, the chip just selects red for that pixel. What are your thoughts on this interface? I was thinking about using some indexed color mode, but that would limit possible future grayscale support[7].
> 
> Hm generally we're trying not to fake stuff in the kernel driver in
> drm. XRGB8888 is an exception because too much stuff blindly assumes
> it exists. I think what we want here is an epaper drm_fourcc.h code,
> at least long-term. But since that's new userspace api it also means
> we need some open source userspace to drive it. I think best approach
> would be to get the basic driver with XRGB8888 emulation merged first,
> and then figure out how to best add the specific epaper formats to
> fully expose the underlying capabilities.

I think I'll rip out the rgb565 part then.

>> The following patches all apply on v5.2. This is my first linux driver, so please be gentle but please do point out all my mistakes :) I'm aware the dt binding doc is still lacking.
> 
> So the great and also somewhat tricky bit is that support code for
> tiny drivers is evolving really quick, so would be good if you can
> rebase onto drm-misc-next from
> https://cgit.freedesktop.org/drm/drm-misc/ There's still tons
> in-flight, but that should at least help. One notable series that
> didn't land yet renames tindydrm to drm/tiny/, so maybe wait for that
> to land (it hopefully should land soon I think).

I'm aware of that and I'll rebase when that's landed.

- Jan