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

[Werner Sembach]

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@xxxxxxxxxxxxxxxxxxx>
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > ---
> > > >     .../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@xxxxxxxxxx>
> 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

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