Re: [PATCH v3 1/1] drm/i915/display: Add quirk to force backlight type on some TUXEDO devices

Hi,

Am 07.08.26 um 04:37 schrieb Kandpal, Suraj:
>> Subject: Re: [PATCH v3 1/1] drm/i915/display: Add quirk to force backlight type
>> on some TUXEDO devices
>>
>> Am 04.08.26 um 21:52 schrieb Werner Sembach:
>>
>>> The display backlight on TUXEDO DX1708 and InsanityBook 15 v1 with
>>> panels AUO 12701 and AUO 12701 must be forced to
>>> INTEL_DP_AUX_BACKLIGHT_ON to be able to control the brightness.
> You need to mention why that is
> "Because the broken VBT in these panels report INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE
> even though the only way to control them is via Intel's backlight interface. Which means'
> the VBT should ideally report INTEL_BACKLIGHT_DISPLAY_DDI in its params"
ok will add
>
>>> This could already be archived via a module parameter, but this patch
> * can already
> * achieved
thanks for spotting
>
>>> adds a quirk to apply this by default on the mentioned devices.
>>>
>>> This patch does not actually test for the exact panels as the id that
>>> is used in the intel_dpcd_quirks list is sadly zeroed on the devices,
>>> but afaik all these devices use try_intel_interface first anyway so
>>> all the quirk does is to add the fallback to try_vesa_interface, so
>>> the behaviour on the devices not needing the quirk and fallback should
>>> functionally stay the same.
>>>
>>> Cc: [email protected]
>>> Signed-off-by: Werner Sembach <[email protected]>
>> Fixes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/15679
>>
>> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/15679
>>
>>> ---
>>>    .../drm/i915/display/intel_dp_aux_backlight.c |  9 ++++++-
>>>    drivers/gpu/drm/i915/display/intel_quirks.c   | 24 +++++++++++++++++++
>>>    drivers/gpu/drm/i915/display/intel_quirks.h   |  1 +
>>>    3 files changed, 33 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>>> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>>> index 266e042e00237..594c59f2d8309 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>>> @@ -41,6 +41,7 @@
>>>    #include "intel_display_types.h"
>>>    #include "intel_dp.h"
>>>    #include "intel_dp_aux_backlight.h"
>>> +#include "intel_quirks.h"
>>>
>>>    /*
>>>     * DP AUX registers for Intel's proprietary HDR backlight interface.
>>> We define @@ -687,11 +688,17 @@ int
>> intel_dp_aux_init_backlight_funcs(struct intel_connector *connector)
>>>    	struct drm_device *dev = connector->base.dev;
>>>    	struct intel_panel *panel = &connector->panel;
>>>    	bool try_intel_interface = false, try_vesa_interface = false;
>>> +	int enable_dpcd_backlight;
>>>
>>>    	/* Check the VBT and user's module parameters to figure out which
>>>    	 * interfaces to probe
>>>    	 */
> This comment need to be moved just above the switch()
ok
>
>>> -	switch (display->params.enable_dpcd_backlight) {
>>> +	enable_dpcd_backlight = display->params.enable_dpcd_backlight;
>>> +	if (enable_dpcd_backlight == INTEL_DP_AUX_BACKLIGHT_AUTO &&
>>> +	    intel_has_dpcd_quirk(intel_dp, QUIRK_ENABLE_DPCD_BACKLIGHT))
>>> +		enable_dpcd_backlight = INTEL_DP_AUX_BACKLIGHT_ON;
> Let's also have a comment here as to why a quirk was used instead of just making VESA the default interface.
> This will help developers avoid going through a series of regressions, fixes and inevitable reverts.

ok i try my best to formulate something

v4 incoming, thanks for the review

Werner Sembach

>
> Regards,
> Suraj Kandpal
>   
>>> +
>>> +	switch (enable_dpcd_backlight) {
>>>    	case INTEL_DP_AUX_BACKLIGHT_OFF:
>>>    		return -ENODEV;
>>>    	case INTEL_DP_AUX_BACKLIGHT_AUTO:
>>> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c
>>> b/drivers/gpu/drm/i915/display/intel_quirks.c
>>> index 33245f44c0d50..89d82364ae45d 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
>>> @@ -100,6 +100,14 @@ static void quirk_disable_psr2(struct intel_display
>> *display)
>>>    	drm_info(display->drm, "PSR2 support not currently available for this
>> setup, applying disable PSR2 quirk\n");
>>>    }
>>>
>>> +static void quirk_enable_dpcd_backlight(struct intel_dp *intel_dp) {
>>> +	struct intel_display *display = to_intel_display(intel_dp);
>>> +
>>> +	intel_set_dpcd_quirk(intel_dp, QUIRK_ENABLE_DPCD_BACKLIGHT);
>>> +	drm_info(display->drm, "Applying Enable DPCD Backlight quirk\n"); }
>>> +
>>>    struct intel_quirk {
>>>    	int device;
>>>    	int subsystem_vendor;
>>> @@ -286,6 +294,22 @@ static const struct intel_dpcd_quirk
>> intel_dpcd_quirks[] = {
>>>    		.sink_oui = SINK_OUI(0x00, 0x22, 0xb9),
>>>    		.hook = quirk_disable_edp_panel_replay,
>>>    	},
>>> +	/* TUXEDO InsanityBook 15 v1 */
>>> +	{
>>> +		.device = 0x591b,
>>> +		.subsystem_vendor = 0x1558,
>>> +		.subsystem_device = 0x9501,
>>> +		.sink_oui = SINK_OUI(0x38, 0xec, 0x11),
>>> +		.hook = quirk_enable_dpcd_backlight,
>>> +	},
>>> +	/* TUXEDO DX1708 */
>>> +	{
>>> +		.device = 0x3e9b,
>>> +		.subsystem_vendor = 0x1558,
>>> +		.subsystem_device = 0x8500,
>>> +		.sink_oui = SINK_OUI(0x38, 0xec, 0x11),
>>> +		.hook = quirk_enable_dpcd_backlight,
>>> +	},
>>>    };
>>>
>>>    void intel_init_quirks(struct intel_display *display) diff --git
>>> a/drivers/gpu/drm/i915/display/intel_quirks.h
>>> b/drivers/gpu/drm/i915/display/intel_quirks.h
>>> index 970a4fe52fafc..4996419ae76bd 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_quirks.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_quirks.h
>>> @@ -23,6 +23,7 @@ enum intel_quirk_id {
>>>    	QUIRK_EDP_LIMIT_RATE_HBR2,
>>>    	QUIRK_DISABLE_EDP_PANEL_REPLAY,
>>>    	QUIRK_DISABLE_PSR2,
>>> +	QUIRK_ENABLE_DPCD_BACKLIGHT,
>>>    };
>>>
>>>    void intel_init_quirks(struct intel_display *display);