Message ID | 20230518031804.3133486-6-matthew.d.roper@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | i915: Move display identification/probing under display/ | expand |
Hi Matt,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Matt-Roper/drm-i915-display-Move-display-device-info-to-header-under-display/20230518-112007
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link: https://lore.kernel.org/r/20230518031804.3133486-6-matthew.d.roper%40intel.com
patch subject: [Intel-gfx] [PATCH 5/5] drm/i915/display: Handle GMD_ID identification in display code
config: i386-randconfig-a004
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/fc14367208dfb37157d27e941b61827dc058c60b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Matt-Roper/drm-i915-display-Move-display-device-info-to-header-under-display/20230518-112007
git checkout fc14367208dfb37157d27e941b61827dc058c60b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305181522.Rsq94cMp-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/i915_driver.c:758:6: warning: variable 'i915' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (ret)
^~~
drivers/gpu/drm/i915/i915_driver.c:849:19: note: uninitialized use occurs here
i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
^~~~
drivers/gpu/drm/i915/i915_utils.h:73:16: note: expanded from macro 'i915_probe_error'
__i915_printk(i915, i915_error_injected() ? KERN_DEBUG : KERN_ERR, \
^~~~
drivers/gpu/drm/i915/i915_driver.c:758:2: note: remove the 'if' if its condition is always false
if (ret)
^~~~~~~~
drivers/gpu/drm/i915/i915_driver.c:754:31: note: initialize the variable 'i915' to silence this warning
struct drm_i915_private *i915;
^
= NULL
1 warning generated.
vim +758 drivers/gpu/drm/i915/i915_driver.c
55ac5a1614f998 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2018-09-05 740
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 741 /**
b01558e56f8486 drivers/gpu/drm/i915/i915_drv.c Janusz Krzysztofik 2019-07-12 742 * i915_driver_probe - setup chip and create an initial config
d2ad3ae4ecf582 drivers/gpu/drm/i915/i915_drv.c Joonas Lahtinen 2016-11-10 743 * @pdev: PCI device
d2ad3ae4ecf582 drivers/gpu/drm/i915/i915_drv.c Joonas Lahtinen 2016-11-10 744 * @ent: matching PCI ID entry
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 745 *
b01558e56f8486 drivers/gpu/drm/i915/i915_drv.c Janusz Krzysztofik 2019-07-12 746 * The driver probe routine has to do several things:
86a1758d751de0 drivers/gpu/drm/i915/i915_driver.c Jani Nikula 2023-04-14 747 * - drive output discovery via intel_display_driver_probe()
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 748 * - initialize the memory manager
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 749 * - allocate initial config memory
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 750 * - setup the DRM framebuffer with the allocated memory
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 751 */
b01558e56f8486 drivers/gpu/drm/i915/i915_drv.c Janusz Krzysztofik 2019-07-12 752 int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 753 {
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 754 struct drm_i915_private *i915;
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 755 int ret;
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 756
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 757 ret = pci_enable_device(pdev);
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 @758 if (ret)
cad3688ff00656 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2017-02-10 759 goto out_fini;
7d87a7f709650b drivers/gpu/drm/i915/i915_drv.c Ville Syrjälä 2014-04-09 760
fc14367208dfb3 drivers/gpu/drm/i915/i915_driver.c Matt Roper 2023-05-17 761 i915 = i915_driver_create(pdev, ent);
fc14367208dfb3 drivers/gpu/drm/i915/i915_driver.c Matt Roper 2023-05-17 762 if (IS_ERR(i915)) {
fc14367208dfb3 drivers/gpu/drm/i915/i915_driver.c Matt Roper 2023-05-17 763 ret = PTR_ERR(i915);
fc14367208dfb3 drivers/gpu/drm/i915/i915_driver.c Matt Roper 2023-05-17 764 goto out_pci_disable;
fc14367208dfb3 drivers/gpu/drm/i915/i915_driver.c Matt Roper 2023-05-17 765 }
fc14367208dfb3 drivers/gpu/drm/i915/i915_driver.c Matt Roper 2023-05-17 766
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 767 ret = i915_driver_early_probe(i915);
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 768 if (ret < 0)
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 769 goto out_pci_disable;
719388e146c34f drivers/gpu/drm/i915/i915_drv.c Damien Lespiau 2015-02-04 770
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 771 disable_rpm_wakeref_asserts(&i915->runtime_pm);
1347f5b46a270d drivers/gpu/drm/i915/i915_drv.c Damien Lespiau 2015-03-17 772
9e859eb9d0f5e3 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-27 773 intel_vgpu_detect(i915);
9e138ea1bdb1d1 drivers/gpu/drm/i915/i915_drv.c Daniele Ceraolo Spurio 2019-06-19 774
bec68cc9ea42d8 drivers/gpu/drm/i915/i915_driver.c Tvrtko Ursulin 2022-03-19 775 ret = intel_gt_probe_all(i915);
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 776 if (ret < 0)
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 777 goto out_runtime_pm_put;
ef11bdb3e00a3f drivers/gpu/drm/i915/i915_drv.c Rodrigo Vivi 2015-10-28 778
bec68cc9ea42d8 drivers/gpu/drm/i915/i915_driver.c Tvrtko Ursulin 2022-03-19 779 ret = i915_driver_mmio_probe(i915);
bec68cc9ea42d8 drivers/gpu/drm/i915/i915_driver.c Tvrtko Ursulin 2022-03-19 780 if (ret < 0)
bec68cc9ea42d8 drivers/gpu/drm/i915/i915_driver.c Tvrtko Ursulin 2022-03-19 781 goto out_tiles_cleanup;
bec68cc9ea42d8 drivers/gpu/drm/i915/i915_driver.c Tvrtko Ursulin 2022-03-19 782
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 783 ret = i915_driver_hw_probe(i915);
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 784 if (ret < 0)
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 785 goto out_cleanup_mmio;
ef11bdb3e00a3f drivers/gpu/drm/i915/i915_drv.c Rodrigo Vivi 2015-10-28 786
86a1758d751de0 drivers/gpu/drm/i915/i915_driver.c Jani Nikula 2023-04-14 787 ret = intel_display_driver_probe_noirq(i915);
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 788 if (ret < 0)
baf54385af7856 drivers/gpu/drm/i915/i915_drv.c Daniel Vetter 2017-06-21 789 goto out_cleanup_hw;
79e539453b34e3 drivers/gpu/drm/i915/i915_drv.c Jesse Barnes 2008-11-07 790
b664259f3fe2c7 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-19 791 ret = intel_irq_install(i915);
b664259f3fe2c7 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-19 792 if (ret)
b664259f3fe2c7 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-19 793 goto out_cleanup_modeset;
b664259f3fe2c7 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-19 794
86a1758d751de0 drivers/gpu/drm/i915/i915_driver.c Jani Nikula 2023-04-14 795 ret = intel_display_driver_probe_nogem(i915);
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 796 if (ret)
b664259f3fe2c7 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-19 797 goto out_cleanup_irq;
b664259f3fe2c7 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-19 798
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 799 ret = i915_gem_init(i915);
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 800 if (ret)
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 801 goto out_cleanup_modeset2;
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 802
f67986b0119c04 drivers/gpu/drm/i915/i915_driver.c Alan Previn 2022-12-08 803 intel_pxp_init(i915);
f67986b0119c04 drivers/gpu/drm/i915/i915_driver.c Alan Previn 2022-12-08 804
86a1758d751de0 drivers/gpu/drm/i915/i915_driver.c Jani Nikula 2023-04-14 805 ret = intel_display_driver_probe(i915);
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 806 if (ret)
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 807 goto out_cleanup_gem;
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 808
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 809 i915_driver_register(i915);
30c964a6cb7bba drivers/gpu/drm/i915/i915_drv.c Robert Beckett 2015-08-28 810
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 811 enable_rpm_wakeref_asserts(&i915->runtime_pm);
30c964a6cb7bba drivers/gpu/drm/i915/i915_drv.c Robert Beckett 2015-08-28 812
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 813 i915_welcome_messages(i915);
27d558a1a0ab07 drivers/gpu/drm/i915/i915_drv.c Michal Wajdeczko 2017-12-21 814
7fb81e9d80738e drivers/gpu/drm/i915/i915_drv.c Daniel Vetter 2020-03-23 815 i915->do_release = true;
7fb81e9d80738e drivers/gpu/drm/i915/i915_drv.c Daniel Vetter 2020-03-23 816
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 817 return 0;
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 818
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 819 out_cleanup_gem:
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 820 i915_gem_suspend(i915);
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 821 i915_gem_driver_remove(i915);
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 822 i915_gem_driver_release(i915);
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 823 out_cleanup_modeset2:
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 824 /* FIXME clean up the error path */
86a1758d751de0 drivers/gpu/drm/i915/i915_driver.c Jani Nikula 2023-04-14 825 intel_display_driver_remove(i915);
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 826 intel_irq_uninstall(i915);
86a1758d751de0 drivers/gpu/drm/i915/i915_driver.c Jani Nikula 2023-04-14 827 intel_display_driver_remove_noirq(i915);
d6843dda38dfa6 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-09-02 828 goto out_cleanup_modeset;
b664259f3fe2c7 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-19 829 out_cleanup_irq:
b664259f3fe2c7 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-19 830 intel_irq_uninstall(i915);
b664259f3fe2c7 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-19 831 out_cleanup_modeset:
86a1758d751de0 drivers/gpu/drm/i915/i915_driver.c Jani Nikula 2023-04-14 832 intel_display_driver_remove_nogem(i915);
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 833 out_cleanup_hw:
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 834 i915_driver_hw_remove(i915);
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 835 intel_memory_regions_driver_release(i915);
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 836 i915_ggtt_driver_release(i915);
4d8151ae5329cf drivers/gpu/drm/i915/i915_drv.c Thomas Hellström 2021-06-01 837 i915_gem_drain_freed_objects(i915);
4d8151ae5329cf drivers/gpu/drm/i915/i915_drv.c Thomas Hellström 2021-06-01 838 i915_ggtt_driver_late_release(i915);
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 839 out_cleanup_mmio:
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 840 i915_driver_mmio_release(i915);
bec68cc9ea42d8 drivers/gpu/drm/i915/i915_driver.c Tvrtko Ursulin 2022-03-19 841 out_tiles_cleanup:
bec68cc9ea42d8 drivers/gpu/drm/i915/i915_driver.c Tvrtko Ursulin 2022-03-19 842 intel_gt_release_all(i915);
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 843 out_runtime_pm_put:
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 844 enable_rpm_wakeref_asserts(&i915->runtime_pm);
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 845 i915_driver_late_release(i915);
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 846 out_pci_disable:
0673ad472b9849 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2016-06-24 847 pci_disable_device(pdev);
cad3688ff00656 drivers/gpu/drm/i915/i915_drv.c Chris Wilson 2017-02-10 848 out_fini:
8eecfb3985e8c8 drivers/gpu/drm/i915/i915_drv.c Jani Nikula 2020-02-11 849 i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
30c964a6cb7bba drivers/gpu/drm/i915/i915_drv.c Robert Beckett 2015-08-28 850 return ret;
30c964a6cb7bba drivers/gpu/drm/i915/i915_drv.c Robert Beckett 2015-08-28 851 }
30c964a6cb7bba drivers/gpu/drm/i915/i915_drv.c Robert Beckett 2015-08-28 852
Hi Matt, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-tip/drm-tip] url: https://github.com/intel-lab-lkp/linux/commits/Matt-Roper/drm-i915-display-Move-display-device-info-to-header-under-display/20230518-112007 base: git://anongit.freedesktop.org/drm/drm-tip drm-tip patch link: https://lore.kernel.org/r/20230518031804.3133486-6-matthew.d.roper%40intel.com patch subject: [Intel-gfx] [PATCH 5/5] drm/i915/display: Handle GMD_ID identification in display code config: i386-randconfig-s001 compiler: gcc-11 (Debian 11.3.0-12) 11.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.4-39-gce1a6720-dirty # https://github.com/intel-lab-lkp/linux/commit/fc14367208dfb37157d27e941b61827dc058c60b git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Matt-Roper/drm-i915-display-Move-display-device-info-to-header-under-display/20230518-112007 git checkout fc14367208dfb37157d27e941b61827dc058c60b # save the config file mkdir build_dir && cp config build_dir/.config make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 olddefconfig make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202305181708.Ze7kK58y-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/i915/display/intel_display_device.c:691:3: sparse: sparse: symbol 'gmdid_display_map' was not declared. Should it be static? vim +/gmdid_display_map +691 drivers/gpu/drm/i915/display/intel_display_device.c 686 687 struct { 688 u16 ver; 689 u16 rel; 690 const struct intel_display_device_info *display; > 691 } gmdid_display_map[] = { 692 { 14, 0, &xe_lpdp_display }, 693 }; 694
On 18.05.2023 05:18, Matt Roper wrote: > For platforms with GMD_ID support (i.e., everything MTL and beyond), > identification of the display IP present should be based on the contents > of the GMD_ID register rather than a PCI devid match. > > Note that since GMD_ID readout requires access to the PCI BAR, a slight > change to the driver init sequence is needed --- pci_enable_device() is > now called before i915_driver_create(). > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > .../drm/i915/display/intel_display_device.c | 64 +++++++++++++++++-- > .../drm/i915/display/intel_display_device.h | 5 +- > drivers/gpu/drm/i915/i915_driver.c | 10 +-- > drivers/gpu/drm/i915/intel_device_info.c | 13 ++-- > 4 files changed, 78 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c > index 78fa522aaf0b..813a2a494082 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_device.c > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c > @@ -6,7 +6,10 @@ > #include <drm/i915_pciids.h> > #include <drm/drm_color_mgmt.h> > #include <linux/mod_devicetable.h> > +#include <linux/pci.h> > > +#include "i915_drv.h" > +#include "i915_reg.h" > #include "intel_display_device.h" > #include "intel_display_power.h" > #include "intel_display_reg_defs.h" > @@ -674,18 +677,69 @@ static const struct pci_device_id intel_display_ids[] = { > INTEL_RPLP_IDS(&xe_lpd_display), > INTEL_DG2_IDS(&xe_hpd_display), > > - /* FIXME: Replace this with a GMD_ID lookup */ > - INTEL_MTL_IDS(&xe_lpdp_display), > + /* > + * Do not add any GMD_ID-based platforms to this list. They will > + * be probed automatically based on the IP version reported by > + * the hardware. > + */ > }; > > +struct { > + u16 ver; > + u16 rel; > + const struct intel_display_device_info *display; > +} gmdid_display_map[] = { > + { 14, 0, &xe_lpdp_display }, > +}; > + > +static const struct intel_display_device_info * > +probe_gmdid_display(struct drm_i915_private *i915, u16 *ver, u16 *rel, u16 *step) > +{ > + struct pci_dev *pdev = to_pci_dev(i915->drm.dev); > + void __iomem *addr; > + u32 val; > + int i; > + > + addr = pci_iomap_range(pdev, 0, i915_mmio_reg_offset(GMD_ID_DISPLAY), sizeof(u32)); > + if (!addr) { > + drm_err(&i915->drm, "Cannot map MMIO BAR to read display GMD_ID\n"); > + return NULL; > + } > + > + val = ioread32(addr); > + pci_iounmap(pdev, addr); > + > + if (val == 0) > + /* Platform doesn't have display */ > + return NULL; > + > + *ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val); > + *rel = REG_FIELD_GET(GMD_ID_RELEASE_MASK, val); > + *step = REG_FIELD_GET(GMD_ID_STEP, val); > + > + for (i = 0; i < ARRAY_SIZE(gmdid_display_map); i++) > + if (*ver == gmdid_display_map[i].ver && > + *rel == gmdid_display_map[i].rel) > + return gmdid_display_map[i].display; > + > + drm_err(&i915->drm, "Unrecognized display IP version %d.%02d; disabling display.\n", > + *ver, *rel); > + return NULL; > +} > + > const struct intel_display_device_info * > -intel_display_device_probe(u16 pci_devid) > +intel_display_device_probe(struct drm_i915_private *i915, bool has_gmdid, > + u16 *gmdid_ver, u16 *gmdid_rel, u16 *gmdid_step) > { > + struct pci_dev *pdev = to_pci_dev(i915->drm.dev); > int i; > > + if (has_gmdid) > + return probe_gmdid_display(i915, gmdid_ver, gmdid_rel, gmdid_step); > + > for (i = 0; i < ARRAY_SIZE(intel_display_ids); i++) { > - if (intel_display_ids[i].device == pci_devid) > - return (struct intel_display_device_info *)intel_display_ids[i].driver_data; > + if (intel_display_ids[i].device == pdev->device) > + return (const struct intel_display_device_info *)intel_display_ids[i].driver_data; > } > > return NULL; > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h > index 0a60ebfaff80..9a344ee36d8c 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_device.h > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h > @@ -80,7 +80,10 @@ struct intel_display_device_info { > } color; > }; > > +struct drm_i915_private; > + > const struct intel_display_device_info * > -intel_display_device_probe(u16 pci_devid); > +intel_display_device_probe(struct drm_i915_private *i915, bool has_gmdid, > + u16 *ver, u16 *rel, u16 *step); > > #endif > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index 522733a89946..d02c602e9a0b 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -754,14 +754,16 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > struct drm_i915_private *i915; > int ret; > > - i915 = i915_driver_create(pdev, ent); > - if (IS_ERR(i915)) > - return PTR_ERR(i915); > - > ret = pci_enable_device(pdev); > if (ret) > goto out_fini; > > + i915 = i915_driver_create(pdev, ent); > + if (IS_ERR(i915)) { > + ret = PTR_ERR(i915); > + goto out_pci_disable; > + } > + > ret = i915_driver_early_probe(i915); > if (ret < 0) > goto out_pci_disable; > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index 9d0b54ba50c1..5f38ff8caac0 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -345,7 +345,6 @@ static void ip_ver_read(struct drm_i915_private *i915, u32 offset, struct intel_ > static void intel_ipver_early_init(struct drm_i915_private *i915) > { > struct intel_runtime_info *runtime = RUNTIME_INFO(i915); > - struct intel_display_runtime_info *display_runtime = DISPLAY_RUNTIME_INFO(i915); > > if (!HAS_GMD_ID(i915)) { > drm_WARN_ON(&i915->drm, RUNTIME_INFO(i915)->graphics.ip.ver > 12); > @@ -366,8 +365,6 @@ static void intel_ipver_early_init(struct drm_i915_private *i915) > RUNTIME_INFO(i915)->graphics.ip.ver = 12; > RUNTIME_INFO(i915)->graphics.ip.rel = 70; > } > - ip_ver_read(i915, i915_mmio_reg_offset(GMD_ID_DISPLAY), > - (struct intel_ip_version *)&display_runtime->ip); > ip_ver_read(i915, i915_mmio_reg_offset(GMD_ID_MEDIA), > &runtime->media.ip); > } > @@ -575,6 +572,7 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, > struct intel_device_info *info; > struct intel_runtime_info *runtime; > struct intel_display_runtime_info *display_runtime; > + u16 ver, rel, step; > > /* Setup the write-once "constant" device info */ > info = mkwrite_device_info(i915); > @@ -585,11 +583,18 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, > memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime)); > > /* Probe display support */ > - info->display = intel_display_device_probe(device_id); > + info->display = intel_display_device_probe(i915, info->has_gmd_id, > + &ver, &rel, &step); > if (info->display) { > display_runtime = DISPLAY_RUNTIME_INFO(i915); > memcpy(display_runtime, &DISPLAY_INFO(i915)->__runtime, > sizeof(*display_runtime)); > + > + if (info->has_gmd_id) { > + display_runtime->ip.ver = ver; > + display_runtime->ip.rel = rel; > + display_runtime->ip.step = step; > + } > } else { > info->display = &no_display; > } Why not embed display stuff into some intel_display_info_create(i915) ? It could be one tiny step further in separating display from i915. It could also allow write ver, rel, step directly into runtime instead of passing them via pointer to local vars and copying. Regards Andrzej
On Thu, May 18, 2023 at 12:44:04PM +0200, Andrzej Hajda wrote: > On 18.05.2023 05:18, Matt Roper wrote: > > For platforms with GMD_ID support (i.e., everything MTL and beyond), > > identification of the display IP present should be based on the contents > > of the GMD_ID register rather than a PCI devid match. > > > > Note that since GMD_ID readout requires access to the PCI BAR, a slight > > change to the driver init sequence is needed --- pci_enable_device() is > > now called before i915_driver_create(). > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > .../drm/i915/display/intel_display_device.c | 64 +++++++++++++++++-- > > .../drm/i915/display/intel_display_device.h | 5 +- > > drivers/gpu/drm/i915/i915_driver.c | 10 +-- > > drivers/gpu/drm/i915/intel_device_info.c | 13 ++-- > > 4 files changed, 78 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c > > index 78fa522aaf0b..813a2a494082 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_device.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c > > @@ -6,7 +6,10 @@ > > #include <drm/i915_pciids.h> > > #include <drm/drm_color_mgmt.h> > > #include <linux/mod_devicetable.h> > > +#include <linux/pci.h> > > +#include "i915_drv.h" > > +#include "i915_reg.h" > > #include "intel_display_device.h" > > #include "intel_display_power.h" > > #include "intel_display_reg_defs.h" > > @@ -674,18 +677,69 @@ static const struct pci_device_id intel_display_ids[] = { > > INTEL_RPLP_IDS(&xe_lpd_display), > > INTEL_DG2_IDS(&xe_hpd_display), > > - /* FIXME: Replace this with a GMD_ID lookup */ > > - INTEL_MTL_IDS(&xe_lpdp_display), > > + /* > > + * Do not add any GMD_ID-based platforms to this list. They will > > + * be probed automatically based on the IP version reported by > > + * the hardware. > > + */ > > }; > > +struct { > > + u16 ver; > > + u16 rel; > > + const struct intel_display_device_info *display; > > +} gmdid_display_map[] = { > > + { 14, 0, &xe_lpdp_display }, > > +}; > > + > > +static const struct intel_display_device_info * > > +probe_gmdid_display(struct drm_i915_private *i915, u16 *ver, u16 *rel, u16 *step) > > +{ > > + struct pci_dev *pdev = to_pci_dev(i915->drm.dev); > > + void __iomem *addr; > > + u32 val; > > + int i; > > + > > + addr = pci_iomap_range(pdev, 0, i915_mmio_reg_offset(GMD_ID_DISPLAY), sizeof(u32)); > > + if (!addr) { > > + drm_err(&i915->drm, "Cannot map MMIO BAR to read display GMD_ID\n"); > > + return NULL; > > + } > > + > > + val = ioread32(addr); > > + pci_iounmap(pdev, addr); > > + > > + if (val == 0) > > + /* Platform doesn't have display */ > > + return NULL; > > + > > + *ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val); > > + *rel = REG_FIELD_GET(GMD_ID_RELEASE_MASK, val); > > + *step = REG_FIELD_GET(GMD_ID_STEP, val); > > + > > + for (i = 0; i < ARRAY_SIZE(gmdid_display_map); i++) > > + if (*ver == gmdid_display_map[i].ver && > > + *rel == gmdid_display_map[i].rel) > > + return gmdid_display_map[i].display; > > + > > + drm_err(&i915->drm, "Unrecognized display IP version %d.%02d; disabling display.\n", > > + *ver, *rel); > > + return NULL; > > +} > > + > > const struct intel_display_device_info * > > -intel_display_device_probe(u16 pci_devid) > > +intel_display_device_probe(struct drm_i915_private *i915, bool has_gmdid, > > + u16 *gmdid_ver, u16 *gmdid_rel, u16 *gmdid_step) > > { > > + struct pci_dev *pdev = to_pci_dev(i915->drm.dev); > > int i; > > + if (has_gmdid) > > + return probe_gmdid_display(i915, gmdid_ver, gmdid_rel, gmdid_step); > > + > > for (i = 0; i < ARRAY_SIZE(intel_display_ids); i++) { > > - if (intel_display_ids[i].device == pci_devid) > > - return (struct intel_display_device_info *)intel_display_ids[i].driver_data; > > + if (intel_display_ids[i].device == pdev->device) > > + return (const struct intel_display_device_info *)intel_display_ids[i].driver_data; > > } > > return NULL; > > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h > > index 0a60ebfaff80..9a344ee36d8c 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_device.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h > > @@ -80,7 +80,10 @@ struct intel_display_device_info { > > } color; > > }; > > +struct drm_i915_private; > > + > > const struct intel_display_device_info * > > -intel_display_device_probe(u16 pci_devid); > > +intel_display_device_probe(struct drm_i915_private *i915, bool has_gmdid, > > + u16 *ver, u16 *rel, u16 *step); > > #endif > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > > index 522733a89946..d02c602e9a0b 100644 > > --- a/drivers/gpu/drm/i915/i915_driver.c > > +++ b/drivers/gpu/drm/i915/i915_driver.c > > @@ -754,14 +754,16 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > struct drm_i915_private *i915; > > int ret; > > - i915 = i915_driver_create(pdev, ent); > > - if (IS_ERR(i915)) > > - return PTR_ERR(i915); > > - > > ret = pci_enable_device(pdev); > > if (ret) > > goto out_fini; > > + i915 = i915_driver_create(pdev, ent); > > + if (IS_ERR(i915)) { > > + ret = PTR_ERR(i915); > > + goto out_pci_disable; > > + } > > + > > ret = i915_driver_early_probe(i915); > > if (ret < 0) > > goto out_pci_disable; > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > > index 9d0b54ba50c1..5f38ff8caac0 100644 > > --- a/drivers/gpu/drm/i915/intel_device_info.c > > +++ b/drivers/gpu/drm/i915/intel_device_info.c > > @@ -345,7 +345,6 @@ static void ip_ver_read(struct drm_i915_private *i915, u32 offset, struct intel_ > > static void intel_ipver_early_init(struct drm_i915_private *i915) > > { > > struct intel_runtime_info *runtime = RUNTIME_INFO(i915); > > - struct intel_display_runtime_info *display_runtime = DISPLAY_RUNTIME_INFO(i915); > > if (!HAS_GMD_ID(i915)) { > > drm_WARN_ON(&i915->drm, RUNTIME_INFO(i915)->graphics.ip.ver > 12); > > @@ -366,8 +365,6 @@ static void intel_ipver_early_init(struct drm_i915_private *i915) > > RUNTIME_INFO(i915)->graphics.ip.ver = 12; > > RUNTIME_INFO(i915)->graphics.ip.rel = 70; > > } > > - ip_ver_read(i915, i915_mmio_reg_offset(GMD_ID_DISPLAY), > > - (struct intel_ip_version *)&display_runtime->ip); > > ip_ver_read(i915, i915_mmio_reg_offset(GMD_ID_MEDIA), > > &runtime->media.ip); > > } > > @@ -575,6 +572,7 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, > > struct intel_device_info *info; > > struct intel_runtime_info *runtime; > > struct intel_display_runtime_info *display_runtime; > > + u16 ver, rel, step; > > /* Setup the write-once "constant" device info */ > > info = mkwrite_device_info(i915); > > @@ -585,11 +583,18 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, > > memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime)); > > /* Probe display support */ > > - info->display = intel_display_device_probe(device_id); > > + info->display = intel_display_device_probe(i915, info->has_gmd_id, > > + &ver, &rel, &step); > > if (info->display) { > > display_runtime = DISPLAY_RUNTIME_INFO(i915); > > memcpy(display_runtime, &DISPLAY_INFO(i915)->__runtime, > > sizeof(*display_runtime)); > > + > > + if (info->has_gmd_id) { > > + display_runtime->ip.ver = ver; > > + display_runtime->ip.rel = rel; > > + display_runtime->ip.step = step; > > + } > > } else { > > info->display = &no_display; > > } > > > Why not embed display stuff into some intel_display_info_create(i915) ? > It could be one tiny step further in separating display from i915. > It could also allow write ver, rel, step directly into runtime instead of > passing them via pointer to local vars and copying. There might definitely be some further refactoring in the future, but at least as things stand now, the device info (including the default values for the runtime-modified items) is a driver-wide read-only constant structure. We always need to copy the __runtime (for device info) and __runtime_defaults (for display device info) into a per-device structure before updating it. If we didn't do that, probing one device would find up changing a shared structure that other devices are trying to use, which would be bad (e.g., if you had two DG2 cards plugged into the same system that had different fusing). Matt > > Regards > Andrzej
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c index 78fa522aaf0b..813a2a494082 100644 --- a/drivers/gpu/drm/i915/display/intel_display_device.c +++ b/drivers/gpu/drm/i915/display/intel_display_device.c @@ -6,7 +6,10 @@ #include <drm/i915_pciids.h> #include <drm/drm_color_mgmt.h> #include <linux/mod_devicetable.h> +#include <linux/pci.h> +#include "i915_drv.h" +#include "i915_reg.h" #include "intel_display_device.h" #include "intel_display_power.h" #include "intel_display_reg_defs.h" @@ -674,18 +677,69 @@ static const struct pci_device_id intel_display_ids[] = { INTEL_RPLP_IDS(&xe_lpd_display), INTEL_DG2_IDS(&xe_hpd_display), - /* FIXME: Replace this with a GMD_ID lookup */ - INTEL_MTL_IDS(&xe_lpdp_display), + /* + * Do not add any GMD_ID-based platforms to this list. They will + * be probed automatically based on the IP version reported by + * the hardware. + */ }; +struct { + u16 ver; + u16 rel; + const struct intel_display_device_info *display; +} gmdid_display_map[] = { + { 14, 0, &xe_lpdp_display }, +}; + +static const struct intel_display_device_info * +probe_gmdid_display(struct drm_i915_private *i915, u16 *ver, u16 *rel, u16 *step) +{ + struct pci_dev *pdev = to_pci_dev(i915->drm.dev); + void __iomem *addr; + u32 val; + int i; + + addr = pci_iomap_range(pdev, 0, i915_mmio_reg_offset(GMD_ID_DISPLAY), sizeof(u32)); + if (!addr) { + drm_err(&i915->drm, "Cannot map MMIO BAR to read display GMD_ID\n"); + return NULL; + } + + val = ioread32(addr); + pci_iounmap(pdev, addr); + + if (val == 0) + /* Platform doesn't have display */ + return NULL; + + *ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val); + *rel = REG_FIELD_GET(GMD_ID_RELEASE_MASK, val); + *step = REG_FIELD_GET(GMD_ID_STEP, val); + + for (i = 0; i < ARRAY_SIZE(gmdid_display_map); i++) + if (*ver == gmdid_display_map[i].ver && + *rel == gmdid_display_map[i].rel) + return gmdid_display_map[i].display; + + drm_err(&i915->drm, "Unrecognized display IP version %d.%02d; disabling display.\n", + *ver, *rel); + return NULL; +} + const struct intel_display_device_info * -intel_display_device_probe(u16 pci_devid) +intel_display_device_probe(struct drm_i915_private *i915, bool has_gmdid, + u16 *gmdid_ver, u16 *gmdid_rel, u16 *gmdid_step) { + struct pci_dev *pdev = to_pci_dev(i915->drm.dev); int i; + if (has_gmdid) + return probe_gmdid_display(i915, gmdid_ver, gmdid_rel, gmdid_step); + for (i = 0; i < ARRAY_SIZE(intel_display_ids); i++) { - if (intel_display_ids[i].device == pci_devid) - return (struct intel_display_device_info *)intel_display_ids[i].driver_data; + if (intel_display_ids[i].device == pdev->device) + return (const struct intel_display_device_info *)intel_display_ids[i].driver_data; } return NULL; diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h index 0a60ebfaff80..9a344ee36d8c 100644 --- a/drivers/gpu/drm/i915/display/intel_display_device.h +++ b/drivers/gpu/drm/i915/display/intel_display_device.h @@ -80,7 +80,10 @@ struct intel_display_device_info { } color; }; +struct drm_i915_private; + const struct intel_display_device_info * -intel_display_device_probe(u16 pci_devid); +intel_display_device_probe(struct drm_i915_private *i915, bool has_gmdid, + u16 *ver, u16 *rel, u16 *step); #endif diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index 522733a89946..d02c602e9a0b 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -754,14 +754,16 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) struct drm_i915_private *i915; int ret; - i915 = i915_driver_create(pdev, ent); - if (IS_ERR(i915)) - return PTR_ERR(i915); - ret = pci_enable_device(pdev); if (ret) goto out_fini; + i915 = i915_driver_create(pdev, ent); + if (IS_ERR(i915)) { + ret = PTR_ERR(i915); + goto out_pci_disable; + } + ret = i915_driver_early_probe(i915); if (ret < 0) goto out_pci_disable; diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 9d0b54ba50c1..5f38ff8caac0 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -345,7 +345,6 @@ static void ip_ver_read(struct drm_i915_private *i915, u32 offset, struct intel_ static void intel_ipver_early_init(struct drm_i915_private *i915) { struct intel_runtime_info *runtime = RUNTIME_INFO(i915); - struct intel_display_runtime_info *display_runtime = DISPLAY_RUNTIME_INFO(i915); if (!HAS_GMD_ID(i915)) { drm_WARN_ON(&i915->drm, RUNTIME_INFO(i915)->graphics.ip.ver > 12); @@ -366,8 +365,6 @@ static void intel_ipver_early_init(struct drm_i915_private *i915) RUNTIME_INFO(i915)->graphics.ip.ver = 12; RUNTIME_INFO(i915)->graphics.ip.rel = 70; } - ip_ver_read(i915, i915_mmio_reg_offset(GMD_ID_DISPLAY), - (struct intel_ip_version *)&display_runtime->ip); ip_ver_read(i915, i915_mmio_reg_offset(GMD_ID_MEDIA), &runtime->media.ip); } @@ -575,6 +572,7 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, struct intel_device_info *info; struct intel_runtime_info *runtime; struct intel_display_runtime_info *display_runtime; + u16 ver, rel, step; /* Setup the write-once "constant" device info */ info = mkwrite_device_info(i915); @@ -585,11 +583,18 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime)); /* Probe display support */ - info->display = intel_display_device_probe(device_id); + info->display = intel_display_device_probe(i915, info->has_gmd_id, + &ver, &rel, &step); if (info->display) { display_runtime = DISPLAY_RUNTIME_INFO(i915); memcpy(display_runtime, &DISPLAY_INFO(i915)->__runtime, sizeof(*display_runtime)); + + if (info->has_gmd_id) { + display_runtime->ip.ver = ver; + display_runtime->ip.rel = rel; + display_runtime->ip.step = step; + } } else { info->display = &no_display; }
For platforms with GMD_ID support (i.e., everything MTL and beyond), identification of the display IP present should be based on the contents of the GMD_ID register rather than a PCI devid match. Note that since GMD_ID readout requires access to the PCI BAR, a slight change to the driver init sequence is needed --- pci_enable_device() is now called before i915_driver_create(). Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- .../drm/i915/display/intel_display_device.c | 64 +++++++++++++++++-- .../drm/i915/display/intel_display_device.h | 5 +- drivers/gpu/drm/i915/i915_driver.c | 10 +-- drivers/gpu/drm/i915/intel_device_info.c | 13 ++-- 4 files changed, 78 insertions(+), 14 deletions(-)