Re: [RFC v1 2/2] platform/x86/amd: Add AMD DPTCi driver

[Antheas Kapenekakis]

On Tue, 3 Mar 2026 at 21:55, Mario Limonciello (AMD) (kernel.org)
<superm1@xxxxxxxxxx> wrote:
>
> + Sasha
>
> On 3/3/2026 2:40 PM, Antheas Kapenekakis wrote:
> > On Tue, 3 Mar 2026 at 21:10, Mario Limonciello (AMD) (kernel.org)
> > <superm1@xxxxxxxxxx> wrote:
> >>
> >> I'll preface this by saying - I don't have a problem with using an AI to
> >> help write a driver, but please disclose that it was done and that in
> >> this case even you haven't closely audited the results.
> >>
> >> I personally would never submit something generated by an LLM that I
> >> didn't audit and add a S-o-b tag to it (asserting I am willing to stand
> >> by the code).
> >>
> >> I'm glad that I found out it was AI written before I started to review
> >> the code, I would have had a lot more candid comments for you.
> >>
> >> There is a lot of weird stuff in this driver that I'm not going to
> >> comment on and nitpick, but I'll leave a few broad strokes things.
> >
> > Of course, to that end, feel free to skip a full review until I get to
> > properly rewriting it.
> >
> > What is the current stance on Co-bys for that? I'm trying to follow
> > the discussion but I missed the news. I can lint it properly next
> > time.
> >
> >  From my perspective, pouring a month into a driver like this without
> > having a firm commitment that it will go somewhere is a bit hard to
> > stomach.
>
> Sure.
>
> I don't need a dedicated tag telling me what tool you wrote it with.  I
> don't care if it was Opus, Gemini or Qwen3.5.  They all can make
> mistakes that need to be audited.
>
> The most important part to me (or any reviewer) is a signal that I
> shouldn't invest more effort reviewing this than you did writing it and
> reviewing it.
>
> My feeling on this kind of RFC this is the most appropriate tag:
>
> Not-signed-off-by: Foo Bar <foo@xxxxxxx>

I did test the driver and go through it multiple times for what it
counts, including fixing all of the checkpatch warnings.

> > This driver does not have a concept of platform profile. The devices
> > by definition do not have presets and users of handhelds are
> > accustomed to a ppt slider (where userspace interpolates for fppt/sppt
> > or sets them the same).
> >
> > We could layer a platform profile on top of it by extending the driver
> > more and adding suggested preselected profiles for low-power,
> > balanced, and performance. Then custom unlocks the sliders. This is
> > the approach I do currently when I hook to the ppd dbus protocol and
> > it works quite well.
> >
> > As for custom profile, it unlocks the firmware attributes in other
> > drivers. The only difference in this one is the naming of the
> > attributes.
>
> I feel for this to be viable in mainline kernel it should be easy to use
> by default with platform profile support.

Fair, I can look into integrating this. I already have preset values
from my userspace implementation.

In that case, I'd match the Legion ABI 1-1 including the names,
perhaps drop the time params but keep tctl - I think lenovo-other is
proposing a name for this, unsure if that merged. Although I am not a
fan of the macro soup in those drivers, which is why this is based on
the samsung driver.

> >> IMV - NO WAY.
> >>
> >> Device matches are mandatory.  I'm not letting a driver like this bind
> >> to any random piece of hardware in the wild.  The thermal design of each
> >> system is different.  Each system needs it's own quirks/table.
> >
> > I need to update the comments but this is gated behind the kconfig
> > flag. In order for SoC/unbound to become available that needs to be
> > on. If it is not, as you see below the driver ejects.
>
> I don't even want a Kconfig flag to allow it to bind to something
> outside of the quirk list for any of this.  if we don't have
> authoritative values to use we shouldn't have anything binding.
>
> If you need to add support for a new system then add a quirk for it.
>

This driver does not bind to anything or autoload unless there is a
DMI match. It is impossible to autoload actually, there is no backing
device that can load via rules other than DMI match. The kconfig flag
just allows to force load the driver with no max values after
recompiling on devices that feature the method.

I'd say it's important for niche cases such as devices with an EC like
the Ally and debugging (such as unlocking the limits on devices that
match). I'd rather keep functionality like this on mainline so that I
don't need to carry a patch for it. I plan to add quirks for new
devices.

In any case, by default the exposed limits for devices are always
based on their DMI match, even if Y is selected (the default mode is
always "device"). "expanded" requires a command line arg if EXPANDED
is N, which allows for a very soft increase. And EXPANDED unlocks the
modes, as I am trying to eliminate cases where modifying the kernel
command line is required, as modifying the kernel command line is an
anti-pattern I am trying to eliminate.

Antheas