diff mbox series

[v2,1/3] drm: Add panel backlight quirks

Message ID 20240623-amdgpu-min-backlight-quirk-v2-1-cecf7f49da9b@weissschuh.net (mailing list archive)
State New, archived
Headers show
Series drm: backlight quirk infrastructure and lower minimum for Framework AMD 13 | expand

Commit Message

Thomas Weißschuh June 23, 2024, 8:51 a.m. UTC
Panels using a PWM-controlled backlight source without an do not have a
standard way to communicate their valid PWM ranges.
On x86 the ranges are read from ACPI through driver-specific tables.
The built-in ranges are not necessarily correct, or may grow stale if an
older device can be retrofitted with newer panels.

Add a quirk infrastructure with which the valid backlight ranges can be
maintained as part of the kernel.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 Documentation/gpu/drm-kms-helpers.rst        |  3 ++
 drivers/gpu/drm/Kconfig                      |  4 ++
 drivers/gpu/drm/Makefile                     |  1 +
 drivers/gpu/drm/drm_panel_backlight_quirks.c | 67 ++++++++++++++++++++++++++++
 include/drm/drm_utils.h                      | 11 +++++
 5 files changed, 86 insertions(+)

Comments

Mario Limonciello June 23, 2024, 8:20 p.m. UTC | #1
On 6/23/2024 03:51, Thomas Weißschuh wrote:
> Panels using a PWM-controlled backlight source without an do not have a
> standard way to communicate their valid PWM ranges.
> On x86 the ranges are read from ACPI through driver-specific tables.
> The built-in ranges are not necessarily correct, or may grow stale if an
> older device can be retrofitted with newer panels.
> 
> Add a quirk infrastructure with which the valid backlight ranges can be
> maintained as part of the kernel.
> 

So I was just talking to some folks in the Linux handheld gaming 
community (added to CC) about an issue they have where they need to know 
the correct panel orientation.  Due to reuse of panels across vendors 
the orientation on one might not be appropriate on another.  The trick 
is then to detect the combo of both the panel and the DMI data.

It's the same "kind" of problem where something advertised in the 
firmware should be ignored but only on a panel + SMBIOS combination.

So I am wondering if what you're proposing here could be more 
generalized.  IE "drm_panel_quirks.c" instead?

Thoughts?

> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>   Documentation/gpu/drm-kms-helpers.rst        |  3 ++
>   drivers/gpu/drm/Kconfig                      |  4 ++
>   drivers/gpu/drm/Makefile                     |  1 +
>   drivers/gpu/drm/drm_panel_backlight_quirks.c | 67 ++++++++++++++++++++++++++++
>   include/drm/drm_utils.h                      | 11 +++++
>   5 files changed, 86 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> index 59cfe8a7a8ba..1998a2675210 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -224,6 +224,9 @@ Panel Helper Reference
>   .. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c
>      :export:
>   
> +.. kernel-doc:: drivers/gpu/drm/drm_panel_backlight_quirks.c
> +   :export:
> +
>   Panel Self Refresh Helper Reference
>   ===================================
>   
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 959b19a04101..50ccb43315bf 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -443,6 +443,10 @@ config DRM_EXPORT_FOR_TESTS
>   config DRM_PANEL_ORIENTATION_QUIRKS
>   	tristate
>   
> +# Separate option as not all DRM drivers use it
> +config DRM_PANEL_BACKLIGHT_QUIRKS
> +	tristate
> +
>   config DRM_LIB_RANDOM
>   	bool
>   	default n
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index f9ca4f8fa6c5..6669913b907e 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -92,6 +92,7 @@ drm-$(CONFIG_DRM_PANIC) += drm_panic.o
>   obj-$(CONFIG_DRM)	+= drm.o
>   
>   obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
> +obj-$(CONFIG_DRM_PANEL_BACKLIGHT_QUIRKS) += drm_panel_backlight_quirks.o
>   
>   #
>   # Memory-management helpers
> diff --git a/drivers/gpu/drm/drm_panel_backlight_quirks.c b/drivers/gpu/drm/drm_panel_backlight_quirks.c
> new file mode 100644
> index 000000000000..a89b5fd1940e
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_panel_backlight_quirks.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/array_size.h>
> +#include <linux/dmi.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <drm/drm_connector.h>
> +#include <drm/drm_utils.h>
> +
> +struct drm_panel_backlight_entry {
> +	struct {
> +		enum dmi_field field;
> +		const char * const value;
> +	} dmi_match;
> +	struct drm_edid_ident ident;
> +	struct drm_panel_backlight_quirk quirk;
> +};
> +
> +static const struct drm_panel_backlight_entry drm_panel_backlight_entries[] = {
> +};
> +
> +static bool drm_panel_backlight_entry_matches(const struct drm_panel_backlight_entry *entry,
> +					      const struct drm_edid *edid)
> +{
> +	if (!dmi_match(entry->dmi_match.field, entry->dmi_match.value))
> +		return false;
> +
> +	if (!drm_edid_match(edid, &entry->ident))
> +		return false;
> +
> +	return true;
> +}
> +
> +/**
> + * drm_get_panel_panel_quirk - Check for panel backlight quirks
> + * @edid: EDID of the panel to check
> + *
> + * This function checks for platform specific (e.g. DMI based) quirks
> + * providing info on backlight control for systems where this cannot be
> + * probed from the hard-/firm-ware.
> + *
> + * Returns:
> + * A struct drm_panel_backlight_quirk if a quirk is found or NULL otherwise.
> + */
> +const struct drm_panel_backlight_quirk *drm_get_panel_backlight_quirk(const struct drm_edid *edid)
> +{
> +	const struct drm_panel_backlight_entry *entry;
> +	size_t i;
> +
> +	if (!IS_ENABLED(CONFIG_DMI))
> +		return NULL;
> +
> +	if (!edid)
> +		return NULL;
> +
> +	for (i = 0; i < ARRAY_SIZE(drm_panel_backlight_entries); i++) {
> +		entry = &drm_panel_backlight_entries[i];
> +
> +		if (drm_panel_backlight_entry_matches(entry, edid))
> +			return &entry->quirk;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL(drm_get_panel_backlight_quirk);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/include/drm/drm_utils.h b/include/drm/drm_utils.h
> index 70775748d243..37cc6de1a01a 100644
> --- a/include/drm/drm_utils.h
> +++ b/include/drm/drm_utils.h
> @@ -11,9 +11,20 @@
>   #define __DRM_UTILS_H__
>   
>   #include <linux/types.h>
> +#include <drm/drm_edid.h>
> +
> +struct drm_panel_backlight_quirk {
> +	struct {
> +		bool pwm_min_brightness:1;
> +	} overrides;
> +
> +	u8 pwm_min_brightness; /* min_brightness/255 of max */
> +};
>   
>   int drm_get_panel_orientation_quirk(int width, int height);
>   
> +const struct drm_panel_backlight_quirk *drm_get_panel_backlight_quirk(const struct drm_edid *edid);
> +
>   signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec);
>   
>   #endif
>
Hans de Goede June 23, 2024, 8:55 p.m. UTC | #2
Hi,

On 6/23/24 10:20 PM, Mario Limonciello wrote:
> On 6/23/2024 03:51, Thomas Weißschuh wrote:
>> Panels using a PWM-controlled backlight source without an do not have a
>> standard way to communicate their valid PWM ranges.
>> On x86 the ranges are read from ACPI through driver-specific tables.
>> The built-in ranges are not necessarily correct, or may grow stale if an
>> older device can be retrofitted with newer panels.
>>
>> Add a quirk infrastructure with which the valid backlight ranges can be
>> maintained as part of the kernel.
>>
> 
> So I was just talking to some folks in the Linux handheld gaming community (added to CC) about an issue they have where they need to know the correct panel orientation.  Due to reuse of panels across vendors the orientation on one might not be appropriate on another.  The trick is then to detect the combo of both the panel and the DMI data.
> 
> It's the same "kind" of problem where something advertised in the firmware should be ignored but only on a panel + SMBIOS combination.
> 
> So I am wondering if what you're proposing here could be more generalized.  IE "drm_panel_quirks.c" instead?
> 
> Thoughts?

Note we already have a quirk mechanism for non upright mounted lcd-panels:

drivers/gpu/drm/drm_panel_orientation_quirks.c

note that the info here is shared with the simpledrm and
efifb drivers, so if the chose is made to extend this then
that needs to be taken into account.

Regards,

Hans





>> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>> ---
>>   Documentation/gpu/drm-kms-helpers.rst        |  3 ++
>>   drivers/gpu/drm/Kconfig                      |  4 ++
>>   drivers/gpu/drm/Makefile                     |  1 +
>>   drivers/gpu/drm/drm_panel_backlight_quirks.c | 67 ++++++++++++++++++++++++++++
>>   include/drm/drm_utils.h                      | 11 +++++
>>   5 files changed, 86 insertions(+)
>>
>> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
>> index 59cfe8a7a8ba..1998a2675210 100644
>> --- a/Documentation/gpu/drm-kms-helpers.rst
>> +++ b/Documentation/gpu/drm-kms-helpers.rst
>> @@ -224,6 +224,9 @@ Panel Helper Reference
>>   .. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c
>>      :export:
>>   +.. kernel-doc:: drivers/gpu/drm/drm_panel_backlight_quirks.c
>> +   :export:
>> +
>>   Panel Self Refresh Helper Reference
>>   ===================================
>>   diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 959b19a04101..50ccb43315bf 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -443,6 +443,10 @@ config DRM_EXPORT_FOR_TESTS
>>   config DRM_PANEL_ORIENTATION_QUIRKS
>>       tristate
>>   +# Separate option as not all DRM drivers use it
>> +config DRM_PANEL_BACKLIGHT_QUIRKS
>> +    tristate
>> +
>>   config DRM_LIB_RANDOM
>>       bool
>>       default n
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index f9ca4f8fa6c5..6669913b907e 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -92,6 +92,7 @@ drm-$(CONFIG_DRM_PANIC) += drm_panic.o
>>   obj-$(CONFIG_DRM)    += drm.o
>>     obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
>> +obj-$(CONFIG_DRM_PANEL_BACKLIGHT_QUIRKS) += drm_panel_backlight_quirks.o
>>     #
>>   # Memory-management helpers
>> diff --git a/drivers/gpu/drm/drm_panel_backlight_quirks.c b/drivers/gpu/drm/drm_panel_backlight_quirks.c
>> new file mode 100644
>> index 000000000000..a89b5fd1940e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_panel_backlight_quirks.c
>> @@ -0,0 +1,67 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/array_size.h>
>> +#include <linux/dmi.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/module.h>
>> +#include <drm/drm_connector.h>
>> +#include <drm/drm_utils.h>
>> +
>> +struct drm_panel_backlight_entry {
>> +    struct {
>> +        enum dmi_field field;
>> +        const char * const value;
>> +    } dmi_match;
>> +    struct drm_edid_ident ident;
>> +    struct drm_panel_backlight_quirk quirk;
>> +};
>> +
>> +static const struct drm_panel_backlight_entry drm_panel_backlight_entries[] = {
>> +};
>> +
>> +static bool drm_panel_backlight_entry_matches(const struct drm_panel_backlight_entry *entry,
>> +                          const struct drm_edid *edid)
>> +{
>> +    if (!dmi_match(entry->dmi_match.field, entry->dmi_match.value))
>> +        return false;
>> +
>> +    if (!drm_edid_match(edid, &entry->ident))
>> +        return false;
>> +
>> +    return true;
>> +}
>> +
>> +/**
>> + * drm_get_panel_panel_quirk - Check for panel backlight quirks
>> + * @edid: EDID of the panel to check
>> + *
>> + * This function checks for platform specific (e.g. DMI based) quirks
>> + * providing info on backlight control for systems where this cannot be
>> + * probed from the hard-/firm-ware.
>> + *
>> + * Returns:
>> + * A struct drm_panel_backlight_quirk if a quirk is found or NULL otherwise.
>> + */
>> +const struct drm_panel_backlight_quirk *drm_get_panel_backlight_quirk(const struct drm_edid *edid)
>> +{
>> +    const struct drm_panel_backlight_entry *entry;
>> +    size_t i;
>> +
>> +    if (!IS_ENABLED(CONFIG_DMI))
>> +        return NULL;
>> +
>> +    if (!edid)
>> +        return NULL;
>> +
>> +    for (i = 0; i < ARRAY_SIZE(drm_panel_backlight_entries); i++) {
>> +        entry = &drm_panel_backlight_entries[i];
>> +
>> +        if (drm_panel_backlight_entry_matches(entry, edid))
>> +            return &entry->quirk;
>> +    }
>> +
>> +    return NULL;
>> +}
>> +EXPORT_SYMBOL(drm_get_panel_backlight_quirk);
>> +
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/drm/drm_utils.h b/include/drm/drm_utils.h
>> index 70775748d243..37cc6de1a01a 100644
>> --- a/include/drm/drm_utils.h
>> +++ b/include/drm/drm_utils.h
>> @@ -11,9 +11,20 @@
>>   #define __DRM_UTILS_H__
>>     #include <linux/types.h>
>> +#include <drm/drm_edid.h>
>> +
>> +struct drm_panel_backlight_quirk {
>> +    struct {
>> +        bool pwm_min_brightness:1;
>> +    } overrides;
>> +
>> +    u8 pwm_min_brightness; /* min_brightness/255 of max */
>> +};
>>     int drm_get_panel_orientation_quirk(int width, int height);
>>   +const struct drm_panel_backlight_quirk *drm_get_panel_backlight_quirk(const struct drm_edid *edid);
>> +
>>   signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec);
>>     #endif
>>
>
Hans de Goede June 24, 2024, 9 a.m. UTC | #3
Hi Thomas,

On 6/23/24 10:51 AM, Thomas Weißschuh wrote:
> Panels using a PWM-controlled backlight source without an do not have a
> standard way to communicate their valid PWM ranges.
> On x86 the ranges are read from ACPI through driver-specific tables.
> The built-in ranges are not necessarily correct, or may grow stale if an
> older device can be retrofitted with newer panels.
> 
> Add a quirk infrastructure with which the valid backlight ranges can be
> maintained as part of the kernel.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  Documentation/gpu/drm-kms-helpers.rst        |  3 ++
>  drivers/gpu/drm/Kconfig                      |  4 ++
>  drivers/gpu/drm/Makefile                     |  1 +
>  drivers/gpu/drm/drm_panel_backlight_quirks.c | 67 ++++++++++++++++++++++++++++
>  include/drm/drm_utils.h                      | 11 +++++
>  5 files changed, 86 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> index 59cfe8a7a8ba..1998a2675210 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -224,6 +224,9 @@ Panel Helper Reference
>  .. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c
>     :export:
>  
> +.. kernel-doc:: drivers/gpu/drm/drm_panel_backlight_quirks.c
> +   :export:
> +
>  Panel Self Refresh Helper Reference
>  ===================================
>  
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 959b19a04101..50ccb43315bf 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -443,6 +443,10 @@ config DRM_EXPORT_FOR_TESTS
>  config DRM_PANEL_ORIENTATION_QUIRKS
>  	tristate
>  
> +# Separate option as not all DRM drivers use it
> +config DRM_PANEL_BACKLIGHT_QUIRKS
> +	tristate
> +
>  config DRM_LIB_RANDOM
>  	bool
>  	default n
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index f9ca4f8fa6c5..6669913b907e 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -92,6 +92,7 @@ drm-$(CONFIG_DRM_PANIC) += drm_panic.o
>  obj-$(CONFIG_DRM)	+= drm.o
>  
>  obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
> +obj-$(CONFIG_DRM_PANEL_BACKLIGHT_QUIRKS) += drm_panel_backlight_quirks.o
>  
>  #
>  # Memory-management helpers
> diff --git a/drivers/gpu/drm/drm_panel_backlight_quirks.c b/drivers/gpu/drm/drm_panel_backlight_quirks.c
> new file mode 100644
> index 000000000000..a89b5fd1940e
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_panel_backlight_quirks.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/array_size.h>
> +#include <linux/dmi.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <drm/drm_connector.h>
> +#include <drm/drm_utils.h>
> +
> +struct drm_panel_backlight_entry {
> +	struct {
> +		enum dmi_field field;
> +		const char * const value;
> +	} dmi_match;

Matching on a single DMI string is not always enough to uniquely identify
a machine. I would change this to a dmi_system_id struct and then add
an array with 2 dmi_system_id structs in drm_panel_backlight_entry_matches()
and copy the struct to the first array entry + zero out the second entry
(terminator) and then use dmi_check_system().

> +	struct drm_edid_ident ident;

Hmm, what about DSI panels? These do not (always) have EDID info AFAIK.

drivers/gpu/drm/drm_panel_orientation_quirks.c is using a resolution match
so as to hopefully not match external screens, but that is also so that it can
be used from efifb / simpledrm and here you really do want to differentiate
between different panels by panel model.

So I guess that the EDId match is fine and if we ever need to match DSI panels
without EDID we figure something out then.

Thinking more about this I have a question about the approach as a whole though,
I'll reply to the cover-letter with this.

Regards,

Hans



> +	struct drm_panel_backlight_quirk quirk;
> +};
> +
> +static const struct drm_panel_backlight_entry drm_panel_backlight_entries[] = {
> +};
> +
> +static bool drm_panel_backlight_entry_matches(const struct drm_panel_backlight_entry *entry,
> +					      const struct drm_edid *edid)
> +{
> +	if (!dmi_match(entry->dmi_match.field, entry->dmi_match.value))
> +		return false;
> +
> +	if (!drm_edid_match(edid, &entry->ident))
> +		return false;
> +
> +	return true;
> +}
> +
> +/**
> + * drm_get_panel_panel_quirk - Check for panel backlight quirks
> + * @edid: EDID of the panel to check
> + *
> + * This function checks for platform specific (e.g. DMI based) quirks
> + * providing info on backlight control for systems where this cannot be
> + * probed from the hard-/firm-ware.
> + *
> + * Returns:
> + * A struct drm_panel_backlight_quirk if a quirk is found or NULL otherwise.
> + */
> +const struct drm_panel_backlight_quirk *drm_get_panel_backlight_quirk(const struct drm_edid *edid)
> +{
> +	const struct drm_panel_backlight_entry *entry;
> +	size_t i;
> +
> +	if (!IS_ENABLED(CONFIG_DMI))
> +		return NULL;
> +
> +	if (!edid)
> +		return NULL;
> +
> +	for (i = 0; i < ARRAY_SIZE(drm_panel_backlight_entries); i++) {
> +		entry = &drm_panel_backlight_entries[i];
> +
> +		if (drm_panel_backlight_entry_matches(entry, edid))
> +			return &entry->quirk;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL(drm_get_panel_backlight_quirk);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/include/drm/drm_utils.h b/include/drm/drm_utils.h
> index 70775748d243..37cc6de1a01a 100644
> --- a/include/drm/drm_utils.h
> +++ b/include/drm/drm_utils.h
> @@ -11,9 +11,20 @@
>  #define __DRM_UTILS_H__
>  
>  #include <linux/types.h>
> +#include <drm/drm_edid.h>
> +
> +struct drm_panel_backlight_quirk {
> +	struct {
> +		bool pwm_min_brightness:1;
> +	} overrides;
> +
> +	u8 pwm_min_brightness; /* min_brightness/255 of max */
> +};
>  
>  int drm_get_panel_orientation_quirk(int width, int height);
>  
> +const struct drm_panel_backlight_quirk *drm_get_panel_backlight_quirk(const struct drm_edid *edid);
> +
>  signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec);
>  
>  #endif
>
Mario Limonciello June 24, 2024, 6:37 p.m. UTC | #4
On 6/23/2024 15:55, Hans de Goede wrote:
> Hi,
> 
> On 6/23/24 10:20 PM, Mario Limonciello wrote:
>> On 6/23/2024 03:51, Thomas Weißschuh wrote:
>>> Panels using a PWM-controlled backlight source without an do not have a
>>> standard way to communicate their valid PWM ranges.
>>> On x86 the ranges are read from ACPI through driver-specific tables.
>>> The built-in ranges are not necessarily correct, or may grow stale if an
>>> older device can be retrofitted with newer panels.
>>>
>>> Add a quirk infrastructure with which the valid backlight ranges can be
>>> maintained as part of the kernel.
>>>
>>
>> So I was just talking to some folks in the Linux handheld gaming community (added to CC) about an issue they have where they need to know the correct panel orientation.  Due to reuse of panels across vendors the orientation on one might not be appropriate on another.  The trick is then to detect the combo of both the panel and the DMI data.
>>
>> It's the same "kind" of problem where something advertised in the firmware should be ignored but only on a panel + SMBIOS combination.
>>
>> So I am wondering if what you're proposing here could be more generalized.  IE "drm_panel_quirks.c" instead?
>>
>> Thoughts?
> 
> Note we already have a quirk mechanism for non upright mounted lcd-panels:
> 
> drivers/gpu/drm/drm_panel_orientation_quirks.c
> 
> note that the info here is shared with the simpledrm and
> efifb drivers, so if the chose is made to extend this then
> that needs to be taken into account.
> 
> Regards,
> 
> Hans

Thanks for sharing.  Totally agree this is this the better way to go for 
what I raised.
Jeff Johnson June 29, 2024, 4:52 a.m. UTC | #5
On 6/23/24 01:51, Thomas Weißschuh wrote:
> Panels using a PWM-controlled backlight source without an do not have a
> standard way to communicate their valid PWM ranges.
> On x86 the ranges are read from ACPI through driver-specific tables.
> The built-in ranges are not necessarily correct, or may grow stale if an
> older device can be retrofitted with newer panels.
> 
> Add a quirk infrastructure with which the valid backlight ranges can be
> maintained as part of the kernel.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
...

> +EXPORT_SYMBOL(drm_get_panel_backlight_quirk);
> +
> +MODULE_LICENSE("GPL");

Missing a MODULE_DESCRIPTION()

This will result in a make W=1 warning
diff mbox series

Patch

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 59cfe8a7a8ba..1998a2675210 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -224,6 +224,9 @@  Panel Helper Reference
 .. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c
    :export:
 
+.. kernel-doc:: drivers/gpu/drm/drm_panel_backlight_quirks.c
+   :export:
+
 Panel Self Refresh Helper Reference
 ===================================
 
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 959b19a04101..50ccb43315bf 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -443,6 +443,10 @@  config DRM_EXPORT_FOR_TESTS
 config DRM_PANEL_ORIENTATION_QUIRKS
 	tristate
 
+# Separate option as not all DRM drivers use it
+config DRM_PANEL_BACKLIGHT_QUIRKS
+	tristate
+
 config DRM_LIB_RANDOM
 	bool
 	default n
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index f9ca4f8fa6c5..6669913b907e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -92,6 +92,7 @@  drm-$(CONFIG_DRM_PANIC) += drm_panic.o
 obj-$(CONFIG_DRM)	+= drm.o
 
 obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
+obj-$(CONFIG_DRM_PANEL_BACKLIGHT_QUIRKS) += drm_panel_backlight_quirks.o
 
 #
 # Memory-management helpers
diff --git a/drivers/gpu/drm/drm_panel_backlight_quirks.c b/drivers/gpu/drm/drm_panel_backlight_quirks.c
new file mode 100644
index 000000000000..a89b5fd1940e
--- /dev/null
+++ b/drivers/gpu/drm/drm_panel_backlight_quirks.c
@@ -0,0 +1,67 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/array_size.h>
+#include <linux/dmi.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_utils.h>
+
+struct drm_panel_backlight_entry {
+	struct {
+		enum dmi_field field;
+		const char * const value;
+	} dmi_match;
+	struct drm_edid_ident ident;
+	struct drm_panel_backlight_quirk quirk;
+};
+
+static const struct drm_panel_backlight_entry drm_panel_backlight_entries[] = {
+};
+
+static bool drm_panel_backlight_entry_matches(const struct drm_panel_backlight_entry *entry,
+					      const struct drm_edid *edid)
+{
+	if (!dmi_match(entry->dmi_match.field, entry->dmi_match.value))
+		return false;
+
+	if (!drm_edid_match(edid, &entry->ident))
+		return false;
+
+	return true;
+}
+
+/**
+ * drm_get_panel_panel_quirk - Check for panel backlight quirks
+ * @edid: EDID of the panel to check
+ *
+ * This function checks for platform specific (e.g. DMI based) quirks
+ * providing info on backlight control for systems where this cannot be
+ * probed from the hard-/firm-ware.
+ *
+ * Returns:
+ * A struct drm_panel_backlight_quirk if a quirk is found or NULL otherwise.
+ */
+const struct drm_panel_backlight_quirk *drm_get_panel_backlight_quirk(const struct drm_edid *edid)
+{
+	const struct drm_panel_backlight_entry *entry;
+	size_t i;
+
+	if (!IS_ENABLED(CONFIG_DMI))
+		return NULL;
+
+	if (!edid)
+		return NULL;
+
+	for (i = 0; i < ARRAY_SIZE(drm_panel_backlight_entries); i++) {
+		entry = &drm_panel_backlight_entries[i];
+
+		if (drm_panel_backlight_entry_matches(entry, edid))
+			return &entry->quirk;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL(drm_get_panel_backlight_quirk);
+
+MODULE_LICENSE("GPL");
diff --git a/include/drm/drm_utils.h b/include/drm/drm_utils.h
index 70775748d243..37cc6de1a01a 100644
--- a/include/drm/drm_utils.h
+++ b/include/drm/drm_utils.h
@@ -11,9 +11,20 @@ 
 #define __DRM_UTILS_H__
 
 #include <linux/types.h>
+#include <drm/drm_edid.h>
+
+struct drm_panel_backlight_quirk {
+	struct {
+		bool pwm_min_brightness:1;
+	} overrides;
+
+	u8 pwm_min_brightness; /* min_brightness/255 of max */
+};
 
 int drm_get_panel_orientation_quirk(int width, int height);
 
+const struct drm_panel_backlight_quirk *drm_get_panel_backlight_quirk(const struct drm_edid *edid);
+
 signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec);
 
 #endif