[PATCH i-g-t] lib: sync PCI ID macros with kernel

[Zbigniew Kempczyński]

On Tue, Nov 12, 2024 at 01:22:47PM +0200, Jani Nikula wrote:
> On Tue, 12 Nov 2024, Zbigniew Kempczyński <zbigniew.kempczynski at intel.com> wrote:
> > On Thu, Nov 07, 2024 at 10:49:30PM +0200, Jani Nikula wrote:
> >> On Thu, 07 Nov 2024, Kamil Konieczny <kamil.konieczny at linux.intel.com> wrote:
> >> > On 2024-11-06 at 10:58:46 -0800, Ngai-Mint Kwan wrote:
> >> >> diff --git a/lib/i915_pciids.h b/lib/pciids.h
> >> >> similarity index 93%
> >> >> rename from lib/i915_pciids.h
> >> >> rename to lib/pciids.h
> >> >> index 3e39d644e..7632507af 100644
> >> >> --- a/lib/i915_pciids.h
> >> >> +++ b/lib/pciids.h
> >> >> @@ -22,30 +22,23 @@
> >> >>   * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> >> >>   * DEALINGS IN THE SOFTWARE.
> >> >>   */
> >> >> -#ifndef _I915_PCIIDS_H
> >> >> -#define _I915_PCIIDS_H
> >> >> -
> >> >> -/*
> >> >> - * A pci_device_id struct {
> >> >> - *	__u32 vendor, device;
> >> >> - *      __u32 subvendor, subdevice;
> >> >> - *	__u32 class, class_mask;
> >> >> - *	kernel_ulong_t driver_data;
> >> >> - * };
> >> >> - * Don't use C99 here because "class" is reserved and we want to
> >> >> - * give userspace flexibility.
> >> >> - */
> >> >> -#define INTEL_VGA_DEVICE(id, info) { \
> >> >> -	0x8086,	id, \
> >> >> -	~0, ~0, \
> >> >> -	0x030000, 0xff0000, \
> >> >> -	(unsigned long) info }
> >> >> -
> >> >> -#define INTEL_QUANTA_VGA_DEVICE(info) { \
> >> >> -	0x8086,	0x16a, \
> >> >> -	0x152d,	0x8990, \
> >> >> -	0x030000, 0xff0000, \
> >> >> -	(unsigned long) info }
> >> >> +#ifndef __PCIIDS_H__
> >> >> +#define __PCIIDS_H__
> >> >> +
> >> >> +#ifdef __KERNEL__
> >> >
> >> > I am not sure if we want a kernel defs, this is for userspace
> >> > so imho this ifdef KERNEL/endif should be deleted. Or write
> >> > a rationale why you included it here, or make it a comment?
> >> > +cc Zbigniew
> >> 
> >> The file comes verbatim from kernel.
> >
> > I have mixed feelings about copying kernel headers here directly
> > if they are not uapi. However __KERNEL__ conditional was added to
> > this header intentionally so copying this file outside the kernel
> > and further reuse in userspace code is harmless. But I would add
> > some explanation to README.md about pciids.h copying procedure
> > for the future.
> 
> We've been doing this for PCI IDs for at least 10 years, and for
> intel_vbt_defs.h for at least 7 years.
> 
> Please feel free to send documentation patches.

I see no __KERNEL__ in current xe_pciids.h and i915_pciids.h so
there's some change from my perspective. intel_vbt_defs.h also
doesn't contain any kernel conditional inside. I think author
of this change should provide additional documentation about this
(not me) - commit message for me is not enough. Till this change
I haven't noticed any __KERNEL__ usage in igt codebase and I think
if it appears it should be documented what's for dead code is
imported to the project.

--
Zbigniew

> 
> 
> BR,
> Jani.
> 
> -- 
> Jani Nikula, Intel