Add LCD character driver: allows interacting with LCD directly from userspace

[protobits]

Summary

Adds a new character driver which exposes LCD interface to userspace via ioctl() commands. This allows using LCD in a more efficient way from userspace, compared to the framebuffer->LCD adapter. It is useful for userspace graphics libraries such as LVGL.

Impact

Add new driver

Testing

Using LVGL

[xiaoxiang781216]

@v01d do you have plan to add put_area/get_area to boost the speed if the hardware support the rectangle based operation?

[protobits]

Thanks for the review, will start addressing your comments.

@v01d do you have plan to add put_area/get_area to boost the speed if the hardware support the rectangle based operation?

I've been trying to think of the best way to propose such a change as I found some resistance in the past when mentioned. There are various options:

1. minimal approach (less advantage): allow putrun() to support npixels > rowsize, would only support contiguous stream of pixels (not area)
2. extended approach: add something like putarea as per your suggestion, takes more advantage of hardware, but extends LCD api
3. ideal (to me): LCD driver should be able to pass custom ioctls to each underlying driver, the problem here is that this driver sits on top of the LCD API and drivers implement that. it would be much better to have all drivers be a lower-half of this driver directly and remove the LCD api altogether. this means NX should use a userspace API for accessing LCDs (framebuffer already has userspace api with mmap()). this of course is the more drastic change

We could go with 2 if there's agreement, but to me we should point towards 3. I think it is better to have less custom APIs in NuttX and use drivers as much as possible.

[protobits]

I just saw the support for "hardware cursor". I'm not entirely sure what it means. You appear to be able to define a starting x,y and area width and height. So that looks like something that could already be enough for many cases. Or is this for something else? There's a "cursor image" which makes me think of a mouse cursor.

[protobits]

Anyway, I realize that this would still require extending putrun to support larger areas. So I think in conclusion the best approach is still (2).

[xiaoxiang781216]

Thanks for the review, will start addressing your comments.

@v01d do you have plan to add put_area/get_area to boost the speed if the hardware support the rectangle based operation?

I've been trying to think of the best way to propose such a change as I found some resistance in the past when mentioned. There are various options:

1. minimal approach (less advantage): allow putrun() to support npixels > rowsize, would only support contiguous stream of pixels (not area)
2. extended approach: add something like putarea as per your suggestion, takes more advantage of hardware, but extends LCD api

The new callback doesn't make the thing too bad, the simulated layer is very easy to add like this:

1. The common code implement putarea and call putrun line by line if the lcd driver implement putrun
2. The common code implement putrun and call putarea with one line if the lcd driver implement putarea

The userspace can call putrun/putarea as they like regardless what the lcd driver really provide. On the other hand, the driver only need implement one of putrun or putarea base on the hardware capability. So both side can utilize old or new inferace smoothly.

3. ideal (to me): LCD driver should be able to pass custom ioctls to each underlying driver,

It's very bad thing that different LCD driver expose the different custom IOCTL for the same rectangle update functionality, which just make the userspace can't write the portable code.

the problem here is that this driver sits on top of the LCD API and drivers implement that. it would be much better to have all drivers be a lower-half of this driver directly and remove the LCD api altogether. this means NX should use a userspace API for accessing LCDs (framebuffer already has userspace api with mmap()). this of course is the more drastic change

Could you explain more about "LCD api" and "userspace API"?

We could go with 2 if there's agreement, but to me we should point towards 3. I think it is better to have less custom APIs in NuttX and use drivers as much as possible.

I just saw the support for "hardware cursor". I'm not entirely sure what it means. You appear to be able to define a starting x,y and area width and height. So that looks like something that could already be enough for many cases. Or is this for something else? There's a "cursor image" which makes me think of a mouse cursor.

Some PC based graphic card support overlay the cursor image on top of the display buffer which release the software to draw the cursor manually(lite overlay for cursor only). LCD is rare to support the hardware cursor, because LCD is frequently used with touch interface and nobody want to see the cursor on the touch screan.

[protobits]

The new callback doesn't make the thing too bad, the simulated layer is very easy to add like this:

1. The common code implement putarea and call putrun line by line if the lcd driver implement putrun

2. The common code implement putrun and call putarea with one line if the lcd driver implement putarea

The userspace can call putrun/putarea as they like regardless what the lcd driver really provide. On the other hand, the driver only need implement one of putrun or putarea base on the hardware capability. So both side can utilize old or new inferace smoothly.

I'm not sure what you mean by simulated layer. Also, what is common code? These are callbacks that needs to be implemented by a driver. So the options for a driver to not implement it are:

• not initialize the pointer (more compatible with existing drivers)
• initialize it and return ENOSUPP (not supported)

This would assume that the caller will have to check if putarea is indeed supported and if not and call putrun instead. Otherwise. we can do what you suggest, to have some part of NuttX supply a "default" implementation of putarea() which simply calls putrun. This makes sense, but since LCD support is simply a header now, there should be some extra handling to have this "default" callback be replaced and I'm not sure where would this happen in the code. Consider that an LCD instance is simply returned by the board's board_lcd_getdev().

So if we can find an easy way to supply this default putarea() implementation I think it would be the best approach. Alternatively, leave the user to deal with this and have it call putrun() instead.

It's very bad thing that different LCD driver expose the different custom IOCTL for the same rectangle update functionality, which just make the userspace can't write the portable code.

I don't think it is a bad design to allow drivers provide custom ioctls(). This is in fact quite commonly done and is a good way to let a driver expose functionality not contemplated in default IOCTLs. However, I wasn't suggesting to leave LCDs implement their own putarea(), that would indeed be a default IOCTL. But consider that each LCD is quite different (for example, you can write by columns or by rows, you can make the pixels wrap around, etc). So I would have a generic putarea() operation which just allows to send a stream of pixels to a rectangle as default and any other variation would be provided by custom ioctl.

Could you explain more about "LCD api" and "userspace API"?

The userspace API is this character driver. The LCD api to me is the callback interface (lcd_dev_s) and the point is that NXgraphics calls into it directly (see nuttx/graphics/nxglib/lcd/nxglib_setpixel.c). I think this is mostly the problem. If NX would be changed to call into this character driver then LCD drivers would be lower-half character drivers and not simply implementing lcd_dev_s callbacks. But I'm not personally interested in going into NX since I don't use it myself and others may not agree to this either.

Some PC based graphic card support overlay the cursor image on top of the display buffer which release the software to draw the cursor manually(lite overlay for cursor only). LCD is rare to support the hardware cursor, because LCD is frequently used with touch interface and nobody want to see the cursor on the touch screan.

Ok, so that isn't of relevance here then.

[xiaoxiang781216]

The new callback doesn't make the thing too bad, the simulated layer is very easy to add like this:

1. The common code implement putarea and call putrun line by line if the lcd driver implement putrun

2. The common code implement putrun and call putarea with one line if the lcd driver implement putarea

The userspace can call putrun/putarea as they like regardless what the lcd driver really provide. On the other hand, the driver only need implement one of putrun or putarea base on the hardware capability. So both side can utilize old or new inferace smoothly.

I'm not sure what you mean by simulated layer. Also, what is common code?

I mean that:
drivers/lcd/lcd_framebuffer.c
drivers/lcd/lcd_dev.c
It's the good place to implment the callback not provided by lcd driver since these two place are the userspace entry point.

These are callbacks that needs to be implemented by a driver. So the options for a driver to not implement it are:

• not initialize the pointer (more compatible with existing drivers)
• initialize it and return ENOSUPP (not supported)

This would assume that the caller will have to check if putarea is indeed supported and if not and call putrun instead. Otherwise. we can do what you suggest, to have some part of NuttX supply a "default" implementation of putarea() which simply calls putrun. This makes sense, but since LCD support is simply a header now, there should be some extra handling to have this "default" callback be replaced and I'm not sure where would this happen in the code. Consider that an LCD instance is simply returned by the board's board_lcd_getdev().

So if we can find an easy way to supply this default putarea() implementation I think it would be the best approach. Alternatively, leave the user to deal with this and have it call putrun() instead.

The best place is in lcd_framebuffer.c and lcd_dev.c.

It's very bad thing that different LCD driver expose the different custom IOCTL for the same rectangle update functionality, which just make the userspace can't write the portable code.

I don't think it is a bad design to allow drivers provide custom ioctls(). This is in fact quite commonly done and is a good way to let a driver expose functionality not contemplated in default IOCTLs. However, I wasn't suggesting to leave LCDs implement their own putarea(), that would indeed be a default IOCTL. But consider that each LCD is quite different (for example, you can write by columns or by rows, you can make the pixels wrap around, etc). So I would have a generic putarea() operation which just allows to send a stream of pixels to a rectangle as default and any other variation would be provided by custom ioctl.

Yes, it's good for custom ioctl which represent a very special functionality found in the rare hardware, but as you mentioned, putarea is generic extension could be found on many hardware and we need standardize it.

Could you explain more about "LCD api" and "userspace API"?

The userspace API is this character driver. The LCD api to me is the callback interface (lcd_dev_s) and the point is that NXgraphics calls into it directly (see nuttx/graphics/nxglib/lcd/nxglib_setpixel.c). I think this is mostly the problem. If NX would be changed to call into this character driver then LCD drivers would be lower-half character drivers and not simply implementing lcd_dev_s callbacks.

No, NXgraphics is different from LVGL in this case. NXgraphics is implemented in the kernel side, lcd_dev_s is native inteface for Nxgrahics just like sensor driver call i2c_master_s or spi_dev_s.

But I'm not personally interested in going into NX since I don't use it myself and others may not agree to this either.

Some PC based graphic card support overlay the cursor image on top of the display buffer which release the software to draw the cursor manually(lite overlay for cursor only). LCD is rare to support the hardware cursor, because LCD is frequently used with touch interface and nobody want to see the cursor on the touch screan.

Ok, so that isn't of relevance here then.

[xiaoxiang781216]

@v01d could you add FAR for ALL pointers I mentioned in the comment?

[protobits]

I mean that:
drivers/lcd/lcd_framebuffer.c
drivers/lcd/lcd_dev.c
It's the good place to implment the callback not provided by lcd driver since these two place are the userspace entry point.

Right, but that means that as you mention:

No, NXgraphics is different from LVGL in this case. NXgraphics is implemented in the kernel side, lcd_dev_s is native inteface for Nxgrahics just like sensor driver call i2c_master_s or spi_dev_s.

NXgraphics will not see putarea() be replaced with the default implementation which uses putrun(). I know putarea() is not used right now in NXgraphics, but you can see that it would not be really a "common place" for all cases, just for when the LCD_DEV driver is enabled and registered.

[protobits]

@v01d could you add FAR for ALL pointers I mentioned in the comment?

Sorry, forgot to push last change

[protobits]

Ok, I just pushed again

[xiaoxiang781216]

I mean that:
drivers/lcd/lcd_framebuffer.c
drivers/lcd/lcd_dev.c
It's the good place to implment the callback not provided by lcd driver since these two place are the userspace entry point.

Right, but that means that as you mention:

No, NXgraphics is different from LVGL in this case. NXgraphics is implemented in the kernel side, lcd_dev_s is native inteface for Nxgrahics just like sensor driver call i2c_master_s or spi_dev_s.

NXgraphics will not see putarea() be replaced with the default implementation which uses putrun(). I know putarea() is not used right now in NXgraphics, but you can see that it would not be really a "common place" for all cases, just for when the LCD_DEV driver is enabled and registered.

we can expose the wrapper functions(e.g. lcd_putrun and lcd_putarea) in lcd.h and replace ->putrun to lcd_putrun inside Nxgraphics. In the furture, we can even optimize Nxgraphics to call lcd_putarea instead.

[protobits]

I mean that:
drivers/lcd/lcd_framebuffer.c
drivers/lcd/lcd_dev.c
It's the good place to implment the callback not provided by lcd driver since these two place are the userspace entry point.

Right, but that means that as you mention:

No, NXgraphics is different from LVGL in this case. NXgraphics is implemented in the kernel side, lcd_dev_s is native inteface for Nxgrahics just like sensor driver call i2c_master_s or spi_dev_s.

NXgraphics will not see putarea() be replaced with the default implementation which uses putrun(). I know putarea() is not used right now in NXgraphics, but you can see that it would not be really a "common place" for all cases, just for when the LCD_DEV driver is enabled and registered.

we can expose the wrapper functions(e.g. lcd_putrun and lcd_putarea) in lcd.h and replace ->putrun to lcd_putrun inside Nxgraphics. In the furture, we can even optimize Nxgraphics to call lcd_putarea instead.

Ok, could be an option, although it may require converting the rest of the interface into functions as well, otherwise it would be a bit ad-hoc to do this just with putrun/putarea.

But I must say that NXgraphics design is kind of limiting the LCD api. I don't see why NXgraphics must be a kernel level system which conditions the LCD subsystem API. It would make much more sense to have NXgraphics in userspace, using userspace interfaces to FB/LCD and then have LCD implemented directly as upper/lower-half character driver. To me is the cleanest approach and simplifies NuttX as a whole.

For now I will then (in separate PR):

• add a putarea callback to interface
• make all drivers set this pointer to zero (with a comment to revisit in case the driver may implement this later)
• expose putarea in this character driver. if the pointer is null, the ioctl will fail

Next step would be to decide if we add the functions you suggest to provide a default putarea() for when the driver does not support it either at the character driver level or at the LCD API level (with added function), or maybe in some other way.

[protobits]

Just pushed all changes, should be good to go

[xiaoxiang781216]

I mean that:
drivers/lcd/lcd_framebuffer.c
drivers/lcd/lcd_dev.c
It's the good place to implment the callback not provided by lcd driver since these two place are the userspace entry point.

Right, but that means that as you mention:

No, NXgraphics is different from LVGL in this case. NXgraphics is implemented in the kernel side, lcd_dev_s is native inteface for Nxgrahics just like sensor driver call i2c_master_s or spi_dev_s.

NXgraphics will not see putarea() be replaced with the default implementation which uses putrun(). I know putarea() is not used right now in NXgraphics, but you can see that it would not be really a "common place" for all cases, just for when the LCD_DEV driver is enabled and registered.

we can expose the wrapper functions(e.g. lcd_putrun and lcd_putarea) in lcd.h and replace ->putrun to lcd_putrun inside Nxgraphics. In the furture, we can even optimize Nxgraphics to call lcd_putarea instead.

Ok, could be an option, although it may require converting the rest of the interface into functions as well, otherwise it would be a bit ad-hoc to do this just with putrun/putarea.

But I must say that NXgraphics design is kind of limiting the LCD api. I don't see why NXgraphics must be a kernel level system which conditions the LCD subsystem API. It would make much more sense to have NXgraphics in userspace, using userspace interfaces to FB/LCD and then have LCD implemented directly as upper/lower-half character driver. To me is the cleanest approach and simplifies NuttX as a whole.

Yes, NXgrahpics could be located inside userspace like XWindow. I guess that it intially put inside kernel to save the IPC cost.

For now I will then (in separate PR):

• add a putarea callback to interface
• make all drivers set this pointer to zero (with a comment to revisit in case the driver may implement this later)
• expose putarea in this character driver. if the pointer is null, the ioctl will fail

Next step would be to decide if we add the functions you suggest to provide a default putarea() for when the driver does not support it either at the character driver level or at the LCD API level (with added function), or maybe in some other way.

Sure.

[xiaoxiang781216]

@w8jcik please take a look.

[protobits]

@xiaoxiang781216 would you mind merging now that all comments are addressed?