Re: [Intel-gfx] [RFC PATCH 4/5] drm/i915/display: prepend connector name to the backlight

From: "Murthy, Arun R" <arun.r.murthy@intel.com>
To: "Nikula, Jani" <jani.nikula@intel.com>,
	"Manna, Animesh" <animesh.manna@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [RFC PATCH 4/5] drm/i915/display: prepend connector name to the backlight
Date: Tue, 21 Jun 2022 06:01:29 +0000	[thread overview]
Message-ID: <DM6PR11MB31778B9F5ACB76E68A804074BAB39@DM6PR11MB3177.namprd11.prod.outlook.com> (raw)
In-Reply-To: <87wndynsat.fsf@intel.com>

> On Fri, 03 Jun 2022, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
> >> On Thu, 02 Jun 2022, Animesh Manna <animesh.manna@intel.com>
> wrote:
> >> > From: Arun R Murthy <arun.r.murthy@intel.com>
> >> >
> >> > With the enablement of dual eDP, there will have to exist two
> >> > entries of backlight sysfs file. In order to avoid sysfs file name
> >> > duplication, the file names are prepended with the connector name.
> >>
> >> Fixed by 20f85ef89d94 ("drm/i915/backlight: use unique backlight
> >> device
> >> names") about a year ago.
> >>
> > This patches checks if the return value is -EEXIST and then acts accordingly,
> but -EEXIST is not returned.
> > struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
> >                                          const char *name,
> >                                          umode_t mode, kuid_t uid, kgid_t gid,
> >                                          loff_t size,
> >                                          const struct kernfs_ops *ops,
> >                                          void *priv, const void *ns,
> >                                          struct lock_class_key *key) {
> >         struct kernfs_node *kn;
> >         unsigned flags;
> >         int rc;
> >
> >         flags = KERNFS_FILE;
> >
> >         kn = kernfs_new_node(parent, name, (mode & S_IALLUGO) | S_IFREG,
> >                              uid, gid, flags);
> >         if (!kn)
> >                 return ERR_PTR(-ENOMEM);
> >
> > So the condition check with not be satisfied and the backlight registration
> will fail for the 2nd backlight device.
> 
> But the file isn't added by kernfs_new_node(), it just allocates the node. See
> the kernfs_add_one() later in __kernfs_create_file().
> 
Moreover now that we will be supporting dual display, wouldn't it
be better to have the same file naming convention for both the
displays?
Without this patch, the first backlight would create an interface
with name intel_backlight and for the second it would create as
"cardXX-XXX-backlight". There wont be any similarities in the
backlight naming convention.
Would it be better to maintain the same naming convention
across the displays?

Thanks and Regards,
Arun R Murthy
--------------------