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

[Mario Limonciello <mario.limonciello@amd.com>]

On 6/24/2025 12:42 AM, Werner Sembach wrote:
> Hi Mario,
> 
> Am 23.06.25 um 21:42 schrieb Limonciello, Mario:
>> 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?
> 
> Sadly our archive is missing this panel + device combination. This patch 
> is based on our install script that sets this fix via boot parameters 
> since the release of these devices.
> 
> So the quirk is field proven, but I can't actively test it anymore.
> 
> Best regards,
> 
> Werner
> 

Do you recall what kernel version you were doing it with?  I'm just 
wondering if AUX brightness control had a bug with such a kernel.

Do you have this panel on some other hardware perhaps?  Or could you 
send a call out to get some testing done?

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