Re: [PATCH 1/6] auxdisplay/ht16k33: Replace use of fb_blank with backlight helper
Hi Thomas,
Thank you for submitting your patch, it looks fine to me.
Reviewed-by: Robin van der Gracht
On Wed, 13 Mar 2024 16:45:00 +0100
Thomas Zimmermann wrote:
> Replace the use of struct backlight_properties.fb_blank with a
> call to backlight_get_brightness(). The helper implement the same
> logic as the driver's function.
>
> Signed-off-by: Thomas Zimmermann
> Cc: Robin van der Gracht
> Cc: Miguel Ojeda
> ---
> drivers/auxdisplay/ht16k33.c | 7 +--
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/auxdisplay/ht16k33.c
> b/drivers/auxdisplay/ht16k33.c index a90430b7d07ba..83db829b97a5e
> 100644 --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -314,14 +314,9 @@ static int ht16k33_initialize(struct
> ht16k33_priv *priv)
> static int ht16k33_bl_update_status(struct backlight_device *bl)
> {
> - int brightness = bl->props.brightness;
> + int brightness = backlight_get_brightness(bl);
> struct ht16k33_priv *priv = bl_get_data(bl);
>
> - if (bl->props.power != FB_BLANK_UNBLANK ||
> - bl->props.fb_blank != FB_BLANK_UNBLANK ||
> - bl->props.state & BL_CORE_FBBLANK)
> - brightness = 0;
> -
> return ht16k33_brightness_set(priv, brightness);
> }
>
Re: [PATCH 02/10] auxdisplay/ht16k33: Remove struct backlight_ops.check_fb
On Mon, 12 Feb 2024 17:16:35 +0100
Thomas Zimmermann wrote:
> The driver sets struct fb_info.bl_dev to the correct backlight
> device. Thus rely on the backlight core code to match backlight
> and framebuffer devices, and remove the extra check_fb function
> from struct backlight_ops.
>
> Signed-off-by: Thomas Zimmermann
> Cc: Robin van der Gracht
> ---
> drivers/auxdisplay/ht16k33.c | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
> index a90430b7d07ba..0a858db32486b 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -325,16 +325,8 @@ static int ht16k33_bl_update_status(struct
> backlight_device *bl)
> return ht16k33_brightness_set(priv, brightness);
> }
>
> -static int ht16k33_bl_check_fb(struct backlight_device *bl, struct fb_info
> *fi)
> -{
> - struct ht16k33_priv *priv = bl_get_data(bl);
> -
> - return (fi == NULL) || (fi->par == priv);
> -}
> -
> static const struct backlight_ops ht16k33_bl_ops = {
> .update_status = ht16k33_bl_update_status,
> - .check_fb = ht16k33_bl_check_fb,
> };
When combined with the previous patch:
[01/10] backlight: Match backlight device against struct fb_info.bl_dev
(I wasn't in the CC)
Acked-By: Robin van der Gracht
Re: [PATCH 10/32] auxdisplay/ht16k33: Initialize fb_ops with fbdev macros
On 2023-11-15 11:19, Thomas Zimmermann wrote:
Initialize the instance of struct fb_ops with fbdev initializer
macros for framebuffers in virtual address space. Set the read/write,
draw and mmap callbacks to the correct implementation and avoid
implicit defaults. Also select the necessary helpers in Kconfig.
Fbdev drivers sometimes rely on the callbacks being NULL for a
default I/O-memory-based implementation to be invoked; hence
requiring the I/O helpers to be built in any case. Setting all
callbacks in all drivers explicitly will allow to make the I/O
helpers optional. This benefits systems that do not use these
functions.
Signed-off-by: Thomas Zimmermann
Cc: Miguel Ojeda
Cc: Robin van der Gracht
---
drivers/auxdisplay/Kconfig | 5 +
drivers/auxdisplay/ht16k33.c | 7 ++-
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 4377e53f8f572..d944d5298eca8 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -167,10 +167,7 @@ config IMG_ASCII_LCD
config HT16K33
tristate "Holtek Ht16K33 LED controller with keyscan"
depends on FB && I2C && INPUT
- select FB_SYS_FOPS
- select FB_SYS_FILLRECT
- select FB_SYS_COPYAREA
- select FB_SYS_IMAGEBLIT
+ select FB_SYSMEM_HELPERS
select INPUT_MATRIXKMAP
select FB_BACKLIGHT
select NEW_LEDS
diff --git a/drivers/auxdisplay/ht16k33.c
b/drivers/auxdisplay/ht16k33.c
index f1716e3ce6a92..2f1dc6b4e2765 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -356,12 +356,9 @@ static int ht16k33_mmap(struct fb_info *info,
struct vm_area_struct *vma)
static const struct fb_ops ht16k33_fb_ops = {
.owner = THIS_MODULE,
- .fb_read = fb_sys_read,
- .fb_write = fb_sys_write,
+ __FB_DEFAULT_SYSMEM_OPS_RDWR,
.fb_blank = ht16k33_blank,
- .fb_fillrect = sys_fillrect,
- .fb_copyarea = sys_copyarea,
- .fb_imageblit = sys_imageblit,
+ __FB_DEFAULT_SYSMEM_OPS_DRAW,
.fb_mmap = ht16k33_mmap,
};
Reviewed-by: Robin van der Gracht
Re: [PATCH 09/32] auxdisplay/ht16k33: Set FBINFO_VIRTFB flag
On 2023-11-15 11:19, Thomas Zimmermann wrote: The ht16k33 driver operates on system memory. Mark the framebuffer accordingly. Helpers operating on the framebuffer memory will test for the presence of this flag. Signed-off-by: Thomas Zimmermann Cc: Miguel Ojeda Cc: Robin van der Gracht --- drivers/auxdisplay/ht16k33.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c index 3a2d883872249..f1716e3ce6a92 100644 --- a/drivers/auxdisplay/ht16k33.c +++ b/drivers/auxdisplay/ht16k33.c @@ -640,6 +640,7 @@ static int ht16k33_fbdev_probe(struct device *dev, struct ht16k33_priv *priv, INIT_DELAYED_WORK(>work, ht16k33_fb_update); fbdev->info->fbops = _fb_ops; + fbdev->info->flags |= FBINFO_VIRTFB; fbdev->info->screen_buffer = fbdev->buffer; fbdev->info->screen_size = HT16K33_FB_SIZE; fbdev->info->fix = ht16k33_fb_fix; Acked-by: Robin van der Gracht
Re: [PATCH 12/15] auxdisplay: ht16k33: Introduce backlight_get_brightness()
Hi Sam, On 2023-01-08 10:29, Sam Ravnborg wrote: Hi Robin. On Sat, Jan 07, 2023 at 10:02:38PM +0100, Miguel Ojeda wrote: On Sat, Jan 7, 2023 at 7:26 PM Sam Ravnborg via B4 Submission Endpoint wrote: > > Introduce backlight_get_brightness() to simplify logic > and avoid direct access to backlight properties. Note: Stephen sent this one too a while ago (with some more details in the commit message, which is always nice); and then he sent yesterday v2 [1] (to mention the functional change with `BL_CORE_SUSPENDED` [2]). Thanks for the pointers. I will try to move forward with Stephen's patches. Anyway, if it goes via drm-misc, feel free to have my: Acked-by: Miguel Ojeda Though it would be nice to have Robin test the change. Robin - can I get your ack to apply Stephen's original v2 patch to drm-misc? done! see: https://lore.kernel.org/lkml/0b16391f997e6ed005a326e4e48f2...@protonic.nl/ - Robin
