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

["Limonciello, Mario" <Mario.Limonciello@amd.com>]

On 6/23/25 2:13 PM, Rodrigo Siqueira wrote:
> On 06/23, Werner Sembach wrote:
>> gentle bump
>>
>> Am 09.04.25 um 18:27 schrieb Werner Sembach:
>>> The display backlight on TUXEDO Polaris AMD Gen2 and Gen3 with panels
>>> BOE 2420 and BOE 2423 must be forced to pwn controlled to be able to
>>> control the brightness.
>>>
>>> This could already be archived via a module parameter, but this patch adds
>>> a quirk to apply this by default on the mentioned device + panel
>>> combinations.
>>>
>>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>>> Cc: stable@vger.kernel.org
>>> ---
>>>    .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++++++++++++-
>>>    1 file changed, 31 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index 39df45f652b32..2bad6274ad8ff 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -1625,11 +1625,13 @@ static bool dm_should_disable_stutter(struct pci_dev *pdev)
>>>    struct amdgpu_dm_quirks {
>>>    	bool aux_hpd_discon;
>>>    	bool support_edp0_on_dp1;
>>> +	bool boe_2420_2423_bl_force_pwm;
>>>    };
>>>    static struct amdgpu_dm_quirks quirk_entries = {
>>>    	.aux_hpd_discon = false,
>>> -	.support_edp0_on_dp1 = false
>>> +	.support_edp0_on_dp1 = false,
>>> +	.boe_2420_2423_bl_force_pwm = false
>>>    };
>>>    static int edp0_on_dp1_callback(const struct dmi_system_id *id)
>>> @@ -1644,6 +1646,12 @@ static int aux_hpd_discon_callback(const struct dmi_system_id *id)
>>>    	return 0;
>>>    }
>>> +static int boe_2420_2423_bl_force_pwm_callback(const struct dmi_system_id *id)
>>> +{
>>> +	quirk_entries.boe_2420_2423_bl_force_pwm = true;
>>> +	return 0;
>>> +}
>>> +
>>>    static const struct dmi_system_id dmi_quirk_table[] = {
>>>    	{
>>>    		.callback = aux_hpd_discon_callback,
>>> @@ -1722,6 +1730,20 @@ static const struct dmi_system_id dmi_quirk_table[] = {
>>>    			DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 665 16 inch G11 Notebook PC"),
>>>    		},
>>>    	},
>>> +	{
>>> +		// TUXEDO Polaris AMD Gen2
>>> +		.callback = boe_2420_2423_bl_force_pwm_callback,
>>> +		.matches = {
>>> +			DMI_MATCH(DMI_BOARD_NAME, "GMxNGxx"),
>>> +		},
>>> +	},
>>> +	{
>>> +		// TUXEDO Polaris AMD Gen3
>>> +		.callback = boe_2420_2423_bl_force_pwm_callback,
>>> +		.matches = {
>>> +			DMI_MATCH(DMI_BOARD_NAME, "GMxZGxx"),
>>> +		},
>>> +	},
>>>    	{}
>>>    	/* TODO: refactor this from a fixed table to a dynamic option */
>>>    };
>>> @@ -3586,6 +3608,7 @@ static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector)
>>>    	struct amdgpu_device *adev;
>>>    	struct drm_luminance_range_info *luminance_range;
>>>    	int min_input_signal_override;
>>> +	u32 panel;
>>>    	if (aconnector->bl_idx == -1 ||
>>>    	    aconnector->dc_link->connector_signal != SIGNAL_TYPE_EDP)
>>> @@ -3610,6 +3633,13 @@ static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector)
>>>    		caps->aux_support = false;
>>>    	else if (amdgpu_backlight == 1)
>>>    		caps->aux_support = true;
>>> +	else if (amdgpu_backlight == -1 &&
>>> +		 quirk_entries.boe_2420_2423_bl_force_pwm) {
>>> +		panel = drm_edid_get_panel_id(aconnector->drm_edid);
>>> +		if (panel == drm_edid_encode_panel_id('B', 'O', 'E', 0x0974) ||
>>> +		    panel == drm_edid_encode_panel_id('B', 'O', 'E', 0x0977))
>>> +			caps->aux_support = false;
>>> +	}
> 
> It lgtm,
> 
> Additionally, I believe this is safe to merge since it only affects a
> specific device. Perhaps display folks would like to include this as
> part of this week's promotion? Anyway, Cc other devs from the display.
> 
> Reviewed-by: Rodrigo Siqueira <siqueira@igalia.com>

That's a bit odd that aux based B/L control wouldn't work.  Are these 
both OLED panels?  What debugging have you done thus far with them?
What kernel base?

Could you repro on 6.16-rc3?

> 
>>>    	if (caps->aux_support)
>>>    		aconnector->dc_link->backlight_control_type = BACKLIGHT_CONTROL_AMD_AUX;
>