Re: [PATCH i-g-t] lib: Pass device fd to gem_mmappable_aperture_size()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>
>Sent: Monday, December 14, 2020 2:15 PM
>To: Chang, Yu bruce; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>Subject: Re: [PATCH i-g-t] lib: Pass device fd to gem_mmappable_aperture_size()
>Quoting Chang, Yu bruce (2020-12-14 21:52:10)
>> 
>> >
>> >From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> >Sent: Monday, December 14, 2020 12:48 PM
>> >To: Chang, Yu bruce; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> >Cc: igt-dev@
>> >Subject: Re: [PATCH i-g-t] lib: Pass device fd to
>> gem_mmappable_aperture_size()
>> > 
>> >Quoting Chang, Yu bruce (2020-12-14 18:45:04)
>> >> +/**
>> >> + * gem_mappable_aperture_size:
>> >> + *
>> >> + * Feature test macro to query the kernel for the mappable gpu aperture
>> size.
>> >> + * This is the area available for GTT memory mappings.
>> >> + *
>> > + * Returns: The mappable gtt address space size.
>> > + */
>> > +uint64_t gem_mappable_aperture_size(int fd)
>> > +{
>> > +       struct pci_device *pci_dev = igt_device_get_pci_device(fd);
>> > 
>> > Does it make sense to eliminate the function intel_get_pci_device() if not
>> > being used anymore? But it can be a separate patch.
>> >
>> >It's still used by tools. The complication there is that we mostly
>> >need to lookup the pci device without loading i915.ko. 
>> >-Chris
>> >
>> 
>> That makes sense.
>> 
>> Then we need to make sure not start from a fix slot to look for GPU device in
>> the intel_get_pci_device() below as
>> it may not work for a discrete GPU as that slot can be a non-vga device but
>> with vendor_id 0x8086.
>> 
>>         pci_dev = pci_device_find_by_slot(0, 0, 2, 0);
>>         if (pci_dev == NULL || pci_dev->vendor_id != 0x8086) {
>> 
>> So, either add extra check to make sure it is VGA class or always use 
>> pci_device_next to search.
>
>It's held true for ~20 years :)
>
>I hear you; for the remaining users, they should probably use the lsgpu
>interface to pick the right device to work on (and remove
>intel_get_pci_device).
>
>tools/intel_audio_dump.c
>tools/intel_backlight.c
>tools/intel_display_poller.c
>tools/intel_forcewaked.c
>tools/intel_gpu_time.c
>tools/intel_gtt.c
>tools/intel_infoframes.c
>tools/intel_lid.c
>tools/intel_panel_fitter.c
>tools/intel_reg.c
>tools/intel_reg_checker.c
>tools/intel_watermark.c
>
>A few of those could even be retired.
>-Chris
>
>

Sounds reasonably to me. the rest of your changes look good to me, and also fix my issue.

Thanks,
Bruce

Reviewed-by: Bruce Chang <yu.bruce.chang@xxxxxxxxx>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux