You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "jturnsek (via GitHub)" <gi...@apache.org> on 2023/09/15 17:41:15 UTC
[GitHub] [nuttx] jturnsek opened a new pull request, #10639: Fixing row_size calculation when bpp is equal 1 in lcd_dev driver
jturnsek opened a new pull request, #10639:
URL: https://github.com/apache/nuttx/pull/10639
## Summary
When using monochromatic LCD (Sharp MEMLCD) with LVGL, planeinfo.bpp is set to 1, thus calculating row_size gives 0, instead of actual row size.
## Impact
lcd_dev driver
## Testing
With LVGL and lcd_dev support on Sharp memlcd driver.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #10639: Fixing row_size calculation when bpp is equal 1 in lcd_dev driver
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #10639:
URL: https://github.com/apache/nuttx/pull/10639#discussion_r1327631425
##########
drivers/lcd/lcd_dev.c:
##########
@@ -118,7 +118,8 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct lcddev_area_s *lcd_area =
(FAR struct lcddev_area_s *)arg;
size_t cols = lcd_area->col_end - lcd_area->col_start + 1;
- size_t row_size = cols * (priv->planeinfo.bpp >> 3);
+ size_t row_size = cols * (priv->planeinfo.bpp > 1 ?
Review Comment:
> based on the code it looks more like `priv->planeinfo.bpp > 8` condition is appropriate, but I didn't dive deep into details
Actually not. LVGL has multiple settings, like 1, 8, 16, 32. 1 is reserved for monochromatic displays, but it uses 1 byte per pixel memory model in buffer (strange yes).
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #10639: Fixing row_size calculation when bpp is equal 1 in lcd_dev driver
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #10639:
URL: https://github.com/apache/nuttx/pull/10639#discussion_r1327657652
##########
drivers/lcd/lcd_dev.c:
##########
@@ -118,7 +118,8 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct lcddev_area_s *lcd_area =
(FAR struct lcddev_area_s *)arg;
size_t cols = lcd_area->col_end - lcd_area->col_start + 1;
- size_t row_size = cols * (priv->planeinfo.bpp >> 3);
+ size_t row_size = cols * (priv->planeinfo.bpp > 1 ?
Review Comment:
I'm fine with a current approach.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #10639: Fixing row_size calculation when bpp is equal 1 in lcd_dev driver
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #10639:
URL: https://github.com/apache/nuttx/pull/10639#discussion_r1327647827
##########
drivers/lcd/lcd_dev.c:
##########
@@ -118,7 +118,8 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct lcddev_area_s *lcd_area =
(FAR struct lcddev_area_s *)arg;
size_t cols = lcd_area->col_end - lcd_area->col_start + 1;
- size_t row_size = cols * (priv->planeinfo.bpp >> 3);
+ size_t row_size = cols * (priv->planeinfo.bpp > 1 ?
Review Comment:
I meant that `8 >> 3` is the same as `1`. So maybe we can go with `((priv->planeinfo.bpp + 7) >> 3)`
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #10639: Fixing row_size calculation when bpp is equal 1 in lcd_dev driver
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #10639:
URL: https://github.com/apache/nuttx/pull/10639#discussion_r1327631425
##########
drivers/lcd/lcd_dev.c:
##########
@@ -118,7 +118,8 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct lcddev_area_s *lcd_area =
(FAR struct lcddev_area_s *)arg;
size_t cols = lcd_area->col_end - lcd_area->col_start + 1;
- size_t row_size = cols * (priv->planeinfo.bpp >> 3);
+ size_t row_size = cols * (priv->planeinfo.bpp > 1 ?
Review Comment:
> based on the code it looks more like `priv->planeinfo.bpp > 8` condition is appropriate, but I didn't dive deep into details
Actually not. LVGL has multiple settings, like 1, 8, 16, 32. 1 is reserved for monochromatic displays, but it uses 1 byte per pixel memory model in buffer (strange yes).
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #10639: Fixing row_size calculation when bpp is equal 1 in lcd_dev driver
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #10639:
URL: https://github.com/apache/nuttx/pull/10639#discussion_r1327631425
##########
drivers/lcd/lcd_dev.c:
##########
@@ -118,7 +118,8 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct lcddev_area_s *lcd_area =
(FAR struct lcddev_area_s *)arg;
size_t cols = lcd_area->col_end - lcd_area->col_start + 1;
- size_t row_size = cols * (priv->planeinfo.bpp >> 3);
+ size_t row_size = cols * (priv->planeinfo.bpp > 1 ?
Review Comment:
Actually not. LVGL has multiple settings, like 1, 8, 16, 32. 1 is reserved for monochromatic displays, but it uses 1 byte per pixel memory model in buffer (strange yes).
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #10639: Fixing row_size calculation when bpp is equal 1 in lcd_dev driver
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #10639:
URL: https://github.com/apache/nuttx/pull/10639#discussion_r1327622049
##########
drivers/lcd/lcd_dev.c:
##########
@@ -118,7 +118,8 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct lcddev_area_s *lcd_area =
(FAR struct lcddev_area_s *)arg;
size_t cols = lcd_area->col_end - lcd_area->col_start + 1;
- size_t row_size = cols * (priv->planeinfo.bpp >> 3);
+ size_t row_size = cols * (priv->planeinfo.bpp > 1 ?
Review Comment:
based on the code it looks more like `priv->planeinfo.bpp > 8` condition is appropriate, but I didn't dive deep into details
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #10639: Fixing row_size calculation when bpp is equal 1 in lcd_dev driver
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #10639:
URL: https://github.com/apache/nuttx/pull/10639#discussion_r1327655013
##########
drivers/lcd/lcd_dev.c:
##########
@@ -118,7 +118,8 @@ static int lcddev_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct lcddev_area_s *lcd_area =
(FAR struct lcddev_area_s *)arg;
size_t cols = lcd_area->col_end - lcd_area->col_start + 1;
- size_t row_size = cols * (priv->planeinfo.bpp >> 3);
+ size_t row_size = cols * (priv->planeinfo.bpp > 1 ?
Review Comment:
> I meant that `8 >> 3` is the same as `1`. So maybe we can go with `((priv->planeinfo.bpp + 7) >> 3)`
It would work also, but I do not know. It is a little bit less readable., don't you think.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 merged pull request #10639: Fixing row_size calculation when bpp is equal 1 in lcd_dev driver
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #10639:
URL: https://github.com/apache/nuttx/pull/10639
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org