Contrast settings and tinydrm

[pfiser]

Hello,

I am developing a tinydrm kernel module for driving a simple monochromatic 128x48 LCD. We opted to go for tinydrm instead of fbdev or fbtft since drm/kms is the future, right?

So far so good. I can already display some graphics, etc.

However I need to control contrast of this LCD and I don't seem to be able to find any good examples or helpers on how to expose contrast settings to userspace?

Do we somehow expose some custom ioctl for that? Seems like a dirty hack...

What is the preferred way to do this?

For example fbtft provides methods for this via set_gamma function pointer in fbtftops:

static struct fbtft_display display = {                                         
        .regwidth = 8,                                                          
        .width = WIDTH,                                                         
        .height = HEIGHT,                                                       
        .gamma_num = 1,                                                         
        .gamma_len = 1,                                                         
        .gamma = "00",                                                          
        .fbtftops = {                                                           
                .write_vmem = write_vmem,                                       
                .init_display = init_display,                                   
                .set_addr_win = set_addr_win,                                   
                .blank = blank,                                                 
                .set_gamma = set_gamma,                                         
        },                                                                      
}; 

Can you help @notro ?

[notro]

AFAICS there is no generic contrast property available. The TV connector has one: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#existing-kms-properties It's not straightforward to add a property now because of the added test requirements: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#testing-and-validation

We opted to go for tinydrm instead of fbdev or fbtft since drm/kms is the future, right?

Indeed if you want to add your driver to Linux mainline (no fbdev drivers accepted), but if the driver will be living out-of-tree, the fbdev subsystem will not go away anytime soon (difficult to say when fbtft will be removed, it won't be allowed to stay in drivers/staging forever).

tinydrm has gone through several changes since it was added, so much so that it has disappeared and the functionality sprinkled over other drm helper modules. The tiny drivers now live in drivers/gpu/drm/tiny since Linux 5.4. I don't expect any big changes to the api for these tiny drivers going forward apart from the usual refactoring churn in the subsystem as a whole.

[pfiser]

Hi @notro !

Thanks for your reply.

AFAICS there is no generic contrast property available. The TV connector has one: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#existing-kms-properties It's not straightforward to add a property now because of the added test requirements: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#testing-and-validation

Maybe the best way then would be to control backlight and contrast by selecting options:

CONFIG_BACKLIGHT_LCD_SUPPORT=y                                                  
CONFIG_LCD_CLASS_DEVICE=y     

and registering lcd_device and backlight_device in our driver probe. I believe those two devices export sysfs attributes which can be used by userspace to control contrast and backlight. Backlight device can additionally be passed directly to tinydrm_enable_backlight() and tinydrm_disable_backlight() when enabling/disabling pipe.

What is your opinion? Would this be good design choice?

Indeed if you want to add your driver to Linux mainline (no fbdev drivers accepted), but if the driver will be living out-of-tree, the fbdev subsystem will not go away anytime soon (difficult to say when fbtft will be removed, it won't be allowed to stay in drivers/staging forever).

This will be out-of-tree LKM. Client specifically requested DRM/KMS interface, hence we went for it. Indeed, driving our LCD with fbdev or fbtft would be much simpler, but nontheless we got it working with tinydrm.

tinydrm has gone through several changes since it was added, so much so that it has disappeared and the functionality sprinkled over other drm helper modules. The tiny drivers now live in drivers/gpu/drm/tiny since Linux 5.4. I don't expect any big changes to the api for these tiny drivers going forward apart from the usual refactoring churn in the subsystem as a whole.

We are still on kernel 4.14, but I don't expect much issues porting our LKM to newer version despite tinydrm API changes in 5.4. and onwards.

BR, Primoz

[notro]

Maybe the best way then would be to control backlight and contrast by selecting options:

CONFIG_BACKLIGHT_LCD_SUPPORT=y                                                  
CONFIG_LCD_CLASS_DEVICE=y     

and registering lcd_device and backlight_device in our driver probe. I believe those two devices export sysfs attributes which can be used by userspace to control contrast and backlight. Backlight device can additionally be passed directly to tinydrm_enable_backlight() and tinydrm_disable_backlight() when enabling/disabling pipe.

What is your opinion? Would this be good design choice?

struct lcd_device seems to be a good option, it has the contrast control. I don't see the need for struct backlight_device though, it's for brightness control. The two of them tackle different kinds of panels.

tinydrm_disable/enable_backlight() doesn't support lcd_device, but no need for that really, since you're not using the mipi_dbi helper, right? Did you use the repaper driver as a template?

DRM is missing proper backlight handling, but it's not straightforward to do since it requires coordination with the various userspace stakeholders to get the api right, and no one has stepped up to do the work yet.

[pfiser]

Hi @notro ,

struct lcd_device seems to be a good option, it has the contrast control. I don't see the need for struct backlight_device though, it's for brightness control. The two of them tackle different kinds of panels.

I will probably use both since I need to control contrast + backlight. Thanks anyways for confirmation we're on good track!

tinydrm_disable/enable_backlight() doesn't support lcd_device, but no need for that really, since you're not using the mipi_dbi helper, right?

True, we're not using mipi_dbi helper. We use custom SPI protocol.

Did you use the repaper driver as a template?

Yes, repaper driver was used as a template. Though we still need to improve our driver. I would like to support partial refresh instead of entire screen and obviously backlight and contrast control are still missing.

DRM is missing proper backlight handling, but it's not straightforward to do since it requires coordination with the various userspace stakeholders to get the api right, and no one has stepped up to do the work yet.

Thanks for this info.

BR, Primoz