On Fri, Sep 12, 2025 at 12:19:21AM -0500, Lucas De Marchi wrote:
> On Tue, Sep 09, 2025 at 09:43:02AM -0500, Lucas De Marchi wrote:
> > On Tue, Sep 09, 2025 at 04:50:41AM +0000, Usyskin, Alexander wrote:
> > > > > +static int mei_lb_component_match(struct device *dev, int
> > > > > subcomponent,
> > > > > + void *data)
> > > > > +{
> > > > > + /*
> > > > > + * This function checks if requester is Intel
> > > > > %PCI_CLASS_DISPLAY_VGA
> > > > or
> > > > > + * %PCI_CLASS_DISPLAY_OTHER device, and checks if the requester
> > > > > is
> > > > the
> > > > > + * grand parent of mei_if i.e. late bind MEI device
> > > > > + */
> > > > > + struct device *base = data;
> > > > > + struct pci_dev *pdev;
> > > > > +
> > > > > + if (!dev)
> > > > > + return 0;
> > > > > +
> > > > > + if (!dev_is_pci(dev))
> > > > > + return 0;
> > > > > +
> > > > > + pdev = to_pci_dev(dev);
> > > > > +
> > > > > + if (pdev->vendor != PCI_VENDOR_ID_INTEL)
> > > > > + return 0;
> > > > > +
> > > > > + if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) &&
> > > > > + pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8))
> > > >
> > > > this doesn't seem right, we should allow other PCI classes. AFAICS this
> > > > check could just be removed and just leave the INTEL_COMPONENT_LB below
> > > > to protect for component match
> > > >
> > > > Lucas De Marchi
> > > >
> > >
> > > The subcomponent is unique only in its own instance of the component
> > > framework.
> > > Or I'm wrong here?
> > > We have to ensure that we have Intel display device, not any other device
> > > to
> > > subcomponent check to work correctly.
> >
> > We are matching by child-parent relationship + the component id. So you
> > have both the mei device and another pci device that added that specific
> > subcomponent and both need to have a common parent. Thinking about
> > another device that would match the parent-child relationship: audio,
> > but audio doesn't add that.
> >
> > what scenario would cause a false match that I'm not seeing?
>
> so, I doesn't seem it would fail any, but it's fine as a sanity check.
> This is in fact very similar to mei_pxp_component_match(). If we are
> going to remove the display check, it could be done later on top, making
> sure not to match what it shouldn't.
>
> So, this looks good to me. I tested this on a Battlemage card
> it's correclty loading the firmware:
>
> xe 0000:03:00.0: [drm:xe_late_bind_init [xe]] Request late binding
> firmware xe/fan_control_8086_e20b_8086_1100.bin
> xe 0000:03:00.0: [drm] Using fan_control firmware from
> xe/fan_control_8086_e20b_8086_1100.bin version 203.0.0.0
> ...
> mei_lb xe.mei-gscfi.768-e2c2afa2-3817-4d19-9d95-06b16b588a5d: bound
> 0000:03:00.0 (ops xe_late_bind_component_ops [xe])
> xe 0000:03:00.0: [drm:xe_late_bind_work [xe]] Load fan_control firmware
> xe 0000:03:00.0: [drm:xe_late_bind_work [xe]] Load fan_control firmware
> successful
>
>
> Reviewed-by: Lucas De Marchi <lucas.demar...@intel.com>
>
> Greg, does this look ok to you now for us to merge through drm?
Greg or Arnd,
ack on getting these 2 mei patches in this series from drm-next trees?
Thanks,
Rodrigo.
>
> thanks
> Lucas De Marchi
>
> >
> > Lucas De Marchi
