michallenc commented on PR #18301:
URL: https://github.com/apache/nuttx/pull/18301#issuecomment-3891740040

   > > My original objection still stands - I don't think creating the 
controller with direct framebuffer support is the right approach, it goes 
against the current practices in LCD driver codes. It's unusable on embedded 
devices with small RAMs (primary NuttX targets) and even in a [violation with 
the 
documentation](https://nuttx.apache.org/docs/latest/components/drivers/special/lcd.html)
   > > > Finally, the LCD screen drivers are usually available at drivers/lcd/ 
and implement the callbacks defined at include/nuttx/lcd/lcd.h
   > > > include/nuttx/lcd/lcd.h provides structures and APIs needed to work 
with LCD screens, whether using the framebuffer adapter or the [LCD Character 
Drivers](https://nuttx.apache.org/docs/latest/components/drivers/special/lcd.html#);
   > > 
   > > 
   > > Though yes, there is the word `usually`. As I mentioned in my review, 
the controller should provide LCD interface and thus support BOTH framebuffer 
approach and LCD chardev approach.
   > > I am not gonna block this if you want to merge it, but adding a flawned 
design to the codebase is a dangerous slippery slope.
   > 
   > @michallenc understood your point, could you please provide some driver 
reference that he could use? And explain which modification he needs to do 
(since he explained doesn't have experience with this kind of driver and is new 
to NuttX) ? Thank you for understanding
   
   Sure, I already mentioned ST7789 controller, among others there are ST7735 
or SSDxxxx controllers. I think the ST7796 driver in this MR is good, it just 
needs the `putrun()`, `getrun()`, `putarea()` instead of direct control by 
framebuffer IOCTL. I know @vrmay23 had valid reasons why do it with framebuffer 
because of API, but that API can be used with LCD char dev as well as I 
described before.
   
   I wouldn't oppose to this if it would be fixable in subsequent patches, but 
that would mean removing `st7796_fbinitialize` and thus breaking boards already 
using it - not as problematic as API change, but still nasty.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to