[Intel-xe] [PATCH 0/4] RFC: Add new device configuration infrastructure to

[Lucas De Marchi]

On Thu, Apr 20, 2023 at 08:44:12PM +0000, Stuart Summers wrote:
>On Thu, 2023-04-20 at 11:53 -0700, Lucas De Marchi wrote:
>> On Thu, Apr 20, 2023 at 12:32:55PM +0300, Jani Nikula wrote:
>> > On Wed, 19 Apr 2023, Stuart Summers <stuart.summers at intel.com>
>> > wrote:
>> > > This is attempting to take the best parts of i915 module
>> > > parameters
>> > > (minus the actual module parameters) and add to xe to allow for
>> > > better
>> > > debuggability and configuration in order to help isolate problems
>> > > on a per-device level instead of global module parameters.
>> > >
>> > > Note that I did review a few options here: configfs (not
>> > > generally
>> > > used by the drm stack), module parameters (we have some negative
>> > > history here), sysfs (not the right approach given the focus on
>> > > user interface here). Debugfs is used in various drm drivers to
>> > > configure various device characteristics. The infrastructure
>> > > being
>> > > presented here has at a high level been present in the i915
>> > > driver
>> > > for some years now, so provides a good starting point for quick
>> > > debug additions without exposing users to some of the challenges
>> > > faced with module parameters in the past.
>> >
>> > Looking into configfs has been on my todo list for a long time.
>> > It's
>> > something that often gets recommended as a replacement to module
>> > parameters, but the documentation as well as the existing examples
>> > in
>> > the kernel are, I think, less than stellar. Basically would require
>> > implementing it and seeing how it actually works.
>> >
>> > In any case, I don't think it should be dismissed with just "not
>> > generally used by the drm stack". A decent implementation could set
>> > the
>> > example going forward.
>> >
>> > The main problem with debugfs is the inability to set the default
>> > values
>> > prior to probing the device. This is where module parameters are
>> > handy,
>> > but they aren't device specific (and, as you note, generally
>> > discouraged).
>> >
>> > Looking at the patches, I'm not sure I understand what the
>> > procedure for
>> > setting the debugfs values before probing the device would be. Can
>> > you
>> > provide an example sequence on the command-line please?
>>
>> I think that for any solution != parameters is:  split the module
>> load
>> and the module bind phases.  I've been using that for a long time
>> since
>> it's a general solution for attaching tracepoints on early probe and
>> it applies to any kernel module (differently than e.g. adding a
>> "nomodeset"
>> parameter to achive similar thing). For a debugging thing, usually
>> for early
>> bring up that would be acceptable I think.
>
>Right I know we've done this many times with i915 without issue. This
>is the way we're configuring these things per device now. But it's
>still a good point about configfs.
>
>Anyway it isn't working with xe today, but that's why I sent this as an
>RFC first to get feedback.

yes, I know it doesn't work. My point is that any solution we've come up
with that is not per-module like parameters, means that when testing, the
user will have to split the module loading from module binding to the device
under test. See the issue I pointed where we discussed a little bit
about the options. The separation between load/bind is the same I
use for tracepoints that I blogged about some time ago
(https://politreco.com/2022/03/tracepoints-and-kernel-modules/)

For the approach with debugfs it would be no different:  first you have
to a) load the module, then b) set the options then c) bind it to the
device. But if you accept the split between load/bind, doing it via
debugfs is basically reinventing configfs.

Lucas De Marchi

>
>I was also thinking the same about debug... really all of these should
>be considered debugging features/functions since the driver should to
>able to autodetect the default settings for each device configuration
>based on various registers and per-platform definitions. Still, let me
>dig into configfs a bit more before making that final determination.
>
>Really appreciate the thoughts here though!
>
>-Stuart
>
>>
>> More context:
>> https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/230
>>
>> Lucas De Marchi