> 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" > > > > This could already be archived via a module parameter, but this patch * can already * achieved > > 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() > > - 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. 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);
