mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mario Limonciello <mario.limonciello@amd.com>
To: Antheas Kapenekakis <lkml@antheas.dev>
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, philm@manjaro.org,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH v1 4/5] drm: panel-backlight-quirks: Add brightness mask quirk
Date: Mon, 25 Aug 2025 16:22:20 -0500	[thread overview]
Message-ID: <b189d2c2-3b50-43c9-bcd8-7b0f353786f7@amd.com> (raw)
In-Reply-To: <CAGwozwGQgnZO0V1eNwbO3w7DyaEnfC+OXD0h6r_ZKLcTS42u9A@mail.gmail.com>

On 8/25/2025 4:17 PM, Antheas Kapenekakis wrote:
> On Mon, 25 Aug 2025 at 20:02, Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> On 8/24/2025 3:02 PM, Antheas Kapenekakis wrote:
>>> Certain OLED devices malfunction on specific brightness levels.
>>> Specifically, when DP_SOURCE_BACKLIGHT_LEVEL is written to with
>>> the first byte being 0x00 and sometimes 0x01, the panel forcibly
>>> turns off until the device sleeps again.
>>>
>>> Below are some examples. This was found by iterating over brighness
>>
>> brightness
>>
>>> ranges while printing DP_SOURCE_BACKLIGHT_LEVEL. It was found that
>>> the screen would malfunction on specific values, and some of them
>>> were collected. Summary examples are found below.
>>>
>>> This quirk was tested by removing the workarounds and iterating
>>> from 0 to 50_000 value ranges with a cadence of 0.2s/it. The
>>> range of the panel is 1000...400_000, so the values were slightly
>>> interpolated during testing. The custom brightness curve added on
>>> 6.15 was disabled.
>>>
>>>    86016:  10101000000000000
>>>    86272:  10101000100000000
>>>    87808:  10101011100000000
>>> 251648: 111101011100000000
>>> 251649: 111101011100000001
>>>
>>>    86144:  10101000010000000
>>>    87809:  10101011100000001
>>> 251650: 111101011100000010
>>>
>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3803
>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>
>> To me this sounds like a panel firmware bug that is best driven with the
>> panel vendor.  But I'm guessing you're reporting it on proudution
>> hardware already in the field right?  In the field it's basically
>> unheard of to update the panel firmware.  The process is generally
>> speaking too dangerous/fragile.
>>
>> So in that case a workaround would make sense.  The actual issue as I'm
>> hearing it is that some fractional brightness values aren't working?
>> The API takes millinits, and I guess this was exposed by increasing the
>> granularity of values that userspace can program recently.
>>
>> It's possible it was there before too, but there are probably "more"
>> values that can hit it.
> 
> Yes, it seems like if the first byte is 0 (always) or 1 (sometimes)
> then the panel turns off. The patch that forces using 100mnits works
> fully in my testing but Phil can still hit values on his device.
> Perhaps at a 1nit or 2 nit accuracy all the values are ok which is why
> Windows works fine
> 
> It has already shipped in 4 SKUs from 3 manufacturers as shown below.
> 
>>
>>> ---
>>>    .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  7 +++++
>>>    .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  6 ++++
>>>    drivers/gpu/drm/drm_panel_backlight_quirks.c  | 29 +++++++++++++++++++
>>>    include/drm/drm_utils.h                       |  1 +
>>>    4 files changed, 43 insertions(+)
>>>
>>> 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 4ad80ae615a2..156f2aae6828 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -3662,6 +3662,9 @@ static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector)
>>>                if (panel_backlight_quirk->min_brightness)
>>>                        caps->min_input_signal =
>>>                                panel_backlight_quirk->min_brightness - 1;
>>> +             if (panel_backlight_quirk->brightness_mask)
>>> +                     caps->brightness_mask =
>>> +                             panel_backlight_quirk->brightness_mask;
>>>        }
>>>    }
>>>
>>> @@ -4862,6 +4865,10 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
>>>        brightness = convert_brightness_from_user(caps, dm->brightness[bl_idx]);
>>>        link = (struct dc_link *)dm->backlight_link[bl_idx];
>>>
>>> +     /* Apply brightness quirk */
>>> +     if (caps->brightness_mask)
>>> +             brightness |= caps->brightness_mask;
>>> +
>>
>> I guess a problem I could see with using a mask is that there are
>> basically a bunch of values that are basically becoming no-op.
> 
> Yeah, 1/3, but those were noops anyway, because 3mnit is not much.
> 
>> An alternative would be to decrease the max value (IE some number
>> smaller than 65535 and scale so userspace doesn't request these "broken"
>> values).
>>
>> I'm not sure it's worth the effort though because you will probably
>> still find some subset of values with this problem.
> 
> The patch that was with the VPE quirk essentially does that and works
> really well. But it is not perfect. That's at 100mnits.
> 
>> The other comment I would say is this is probably very specific to AMD
>> and the millinit based brightness API; it might be better to keep the
>> quirk localized to amdgpu.  I also talked to Phil offline about this and
>> he's got a draft patch that helps a similar system he's seeing this on
>> (presumably) with another panel.
>>
>> I think it's worth getting that patch onto the list and we can weigh out
>> the alternatives.
> 
> I think it is the same panel spec. Perhaps it is a different factory,
> but all affected devices are 7in OLED 1080p portrait panels with HDR.
> I sent this patch before I cleaned it up fully because Phil wanted to
> test it. It seems to work well. I think he's looking into implementing
> the same quirk essentially.
> 
> I would not say it is specific to AMD per say. The bug happens in the
> panel after being sent a command to set the brightness through a
> shared drm helper. The same panel on intel with the same value would
> have the same problem.

Does Intel also have millinits brightness control?  I wasn't sure.

> Perhaps not depending on the backlight code
> that preprocesses the values. The drm_panel_backlight_quirks is used
> only by amd currently in any case. And this quirk system allows
> specifying EDID information, where the DRM code one that you did did
> not. I only saw that after I had made this series.
> 
> Antheas

OK thanks.  As you and Phil both have affected devices that are very 
similar result get offline with Phil and come together with a new patch 
series that works for both of you via quirks.

> 
> 
>>>        /* Change brightness based on AUX property */
>>>        mutex_lock(&dm->dc_lock);
>>>        if (dm->dc->caps.ips_support && dm->dc->ctx->dmub_srv->idle_allowed) {
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>>> index b937da0a4e4a..340f9b5f68eb 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>>> @@ -200,6 +200,12 @@ struct amdgpu_dm_backlight_caps {
>>>         * @aux_support: Describes if the display supports AUX backlight.
>>>         */
>>>        bool aux_support;
>>> +     /**
>>> +      * @brightness_mask: After deriving brightness, or it with this mask.
>>> +      * This is used to workaround panels that have issues with certain
>>> +      * brightness values.
>>> +      */
>>> +     u32 brightness_mask;
>>>        /**
>>>         * @ac_level: the default brightness if booted on AC
>>>         */
>>> diff --git a/drivers/gpu/drm/drm_panel_backlight_quirks.c b/drivers/gpu/drm/drm_panel_backlight_quirks.c
>>> index 3d386a96e50e..78c430b07d6a 100644
>>> --- a/drivers/gpu/drm/drm_panel_backlight_quirks.c
>>> +++ b/drivers/gpu/drm/drm_panel_backlight_quirks.c
>>> @@ -45,6 +45,35 @@ static const struct drm_get_panel_backlight_quirk drm_panel_min_backlight_quirks
>>>                .ident.name = "NE135A1M-NY1",
>>>                .quirk = { .min_brightness = 1, },
>>>        },
>>> +     /* Have OLED Panels with brightness issue when last byte is 0/1 */
>>> +     {
>>> +             .dmi_match.field = DMI_SYS_VENDOR,
>>> +             .dmi_match.value = "AYANEO",
>>> +             .dmi_match_other.field = DMI_PRODUCT_NAME,
>>> +             .dmi_match_other.value = "AYANEO 3",
>>> +             .quirk = { .brightness_mask = 3, },
>>> +     },
>>> +     {
>>> +             .dmi_match.field = DMI_SYS_VENDOR,
>>> +             .dmi_match.value = "ZOTAC",
>>> +             .dmi_match_other.field = DMI_BOARD_NAME,
>>> +             .dmi_match_other.value = "G0A1W",
>>> +             .quirk = { .brightness_mask = 3, },
>>> +     },
>>> +     {
>>> +             .dmi_match.field = DMI_SYS_VENDOR,
>>> +             .dmi_match.value = "ONE-NETBOOK",
>>> +             .dmi_match_other.field = DMI_PRODUCT_NAME,
>>> +             .dmi_match_other.value = "ONEXPLAYER F1Pro",
>>> +             .quirk = { .brightness_mask = 3, },
>>> +     },
>>> +     {
>>> +             .dmi_match.field = DMI_SYS_VENDOR,
>>> +             .dmi_match.value = "ONE-NETBOOK",
>>> +             .dmi_match_other.field = DMI_PRODUCT_NAME,
>>> +             .dmi_match_other.value = "ONEXPLAYER F1 EVA-02",
>>> +             .quirk = { .brightness_mask = 3, },
>>> +     }
>>>    };
>>>
>>>    static bool drm_panel_min_backlight_quirk_matches(
>>> diff --git a/include/drm/drm_utils.h b/include/drm/drm_utils.h
>>> index 82eeee4a58ab..6a46f755daba 100644
>>> --- a/include/drm/drm_utils.h
>>> +++ b/include/drm/drm_utils.h
>>> @@ -18,6 +18,7 @@ int drm_get_panel_orientation_quirk(int width, int height);
>>>
>>>    struct drm_panel_backlight_quirk {
>>>        u16 min_brightness;
>>> +     u32 brightness_mask;
>>>    };
>>>
>>>    const struct drm_panel_backlight_quirk *
>>
>>
> 


  reply	other threads:[~2025-08-25 21:22 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-24 20:01 [PATCH v1 0/5] drm: panel-backlight-quirks: Do partial refactor and apply OLED fix Antheas Kapenekakis
2025-08-24 20:01 ` [PATCH v1 1/5] drm: panel-backlight-quirks: Make ident optional Antheas Kapenekakis
2025-08-24 20:01 ` [PATCH v1 2/5] drm: panel-backlight-quirks: Convert brightness quirk to generic structure Antheas Kapenekakis
2025-08-25 17:53   ` Mario Limonciello
2025-08-26 12:29   ` Philip Müller
2025-08-24 20:02 ` [PATCH v1 3/5] drm: panel-backlight-quirks: Add secondary DMI match Antheas Kapenekakis
2025-08-26 12:29   ` Philip Müller
2025-08-24 20:02 ` [PATCH v1 4/5] drm: panel-backlight-quirks: Add brightness mask quirk Antheas Kapenekakis
2025-08-25 18:02   ` Mario Limonciello
2025-08-25 21:17     ` Antheas Kapenekakis
2025-08-25 21:22       ` Mario Limonciello [this message]
2025-08-25 21:26         ` Antheas Kapenekakis
2025-08-26 12:28   ` Philip Müller
2025-08-24 20:02 ` [PATCH v1 5/5] drm: panel-backlight-quirks: Add Steam Decks Antheas Kapenekakis
2025-08-25 16:47   ` Mario Limonciello
2025-08-25 21:02     ` Antheas Kapenekakis
2025-08-25 21:05       ` Mario Limonciello
2025-08-25 21:20         ` Antheas Kapenekakis
2025-08-26 11:38   ` Robert Beckett
2025-08-25 16:45 ` [PATCH v1 0/5] drm: panel-backlight-quirks: Do partial refactor and apply OLED fix Mario Limonciello

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b189d2c2-3b50-43c9-bcd8-7b0f353786f7@amd.com \
    --to=mario.limonciello@amd.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkml@antheas.dev \
    --cc=philm@manjaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®