diff mbox series

[v1,2/4] drm/i915: Add missing mask when reading GEN12_DSMBASE

Message ID 20220915-stolen-v1-2-117c5f295bb2@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Improvements to stolen memory setup | expand

Commit Message

Lucas De Marchi Sept. 15, 2022, 8:39 p.m. UTC 
DSMBASE register is defined so BDSM bitfield contains the bits 63 to 20
of the base address of stolen. For the supported platforms bits 0-19 are
zero but that may not be true in future. Add the missing mask.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Comments

Caz Yokoyama Sept. 15, 2022, 9:03 p.m. UTC | #1 
Reviewed-by:  Caz Yokoyama <caz@caztech.com>
Better to be safe.


On Thu, Sep 15, 2022 at 1:40 PM Lucas De Marchi <lucas.demarchi@intel.com>
wrote:

> DSMBASE register is defined so BDSM bitfield contains the bits 63 to 20
> of the base address of stolen. For the supported platforms bits 0-19 are
> zero but that may not be true in future. Add the missing mask.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index 42f4769bb4ac..c34065fe2ecc 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -814,7 +814,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private
> *i915, u16 type,
>                 return ERR_PTR(-ENXIO);
>
>         /* Use DSM base address instead for stolen memory */
> -       dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE);
> +       dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) &
> GEN12_BDSM_MASK;
>         if (IS_DG1(uncore->i915)) {
>                 lmem_size = pci_resource_len(pdev, GEN12_LMEM_BAR);
>                 if (WARN_ON(lmem_size < dsm_base))
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 1a9bd829fc7e..0301874c76ba 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7953,6 +7953,7 @@ enum skl_power_gate {
>
>  #define GEN12_GSMBASE                  _MMIO(0x108100)
>  #define GEN12_DSMBASE                  _MMIO(0x1080C0)
> +#define   GEN12_BDSM_MASK              GENMASK(63, 20)
>
>  #define XEHP_CLOCK_GATE_DIS            _MMIO(0x101014)
>  #define   SGSI_SIDECLK_DIS             REG_BIT(17)
>
> --
> b4 0.10.0-dev-bbe61
>
kernel test robot Sept. 16, 2022, 12:33 a.m. UTC | #2 
Hi Lucas,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bb4f6b2281b11b009210f62eecd291f7b75c1e85]

url:    https://github.com/intel-lab-lkp/linux/commits/Lucas-De-Marchi/drm-i915-Improvements-to-stolen-memory-setup/20220916-044155
base:   bb4f6b2281b11b009210f62eecd291f7b75c1e85
config: i386-defconfig
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/b0c14c92efecabadc483675b606c1ce109cfa415
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Lucas-De-Marchi/drm-i915-Improvements-to-stolen-memory-setup/20220916-044155
        git checkout b0c14c92efecabadc483675b606c1ce109cfa415
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/bitops.h:6,
                    from include/linux/kernel.h:22,
                    from arch/x86/include/asm/percpu.h:27,
                    from arch/x86/include/asm/current.h:6,
                    from include/linux/mutex.h:14,
                    from drivers/gpu/drm/i915/gem/i915_gem_stolen.c:8:
   drivers/gpu/drm/i915/gem/i915_gem_stolen.c: In function 'i915_gem_stolen_lmem_setup':
>> include/linux/bits.h:36:18: error: right shift count is negative [-Werror=shift-count-negative]
      36 |          (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
         |                  ^~
   include/linux/bits.h:38:38: note: in expansion of macro '__GENMASK'
      38 |         (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
         |                                      ^~~~~~~~~
   drivers/gpu/drm/i915/i915_reg.h:7956:41: note: in expansion of macro 'GENMASK'
    7956 | #define   GEN12_BDSM_MASK               GENMASK(63, 20)
         |                                         ^~~~~~~
   drivers/gpu/drm/i915/gem/i915_gem_stolen.c:817:65: note: in expansion of macro 'GEN12_BDSM_MASK'
     817 |         dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & GEN12_BDSM_MASK;
         |                                                                 ^~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors


vim +36 include/linux/bits.h

295bcca84916cb5 Rikard Falkeborn 2020-04-06  33  
295bcca84916cb5 Rikard Falkeborn 2020-04-06  34  #define __GENMASK(h, l) \
95b980d62d52c4c Masahiro Yamada  2019-07-16  35  	(((~UL(0)) - (UL(1) << (l)) + 1) & \
95b980d62d52c4c Masahiro Yamada  2019-07-16 @36  	 (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
295bcca84916cb5 Rikard Falkeborn 2020-04-06  37  #define GENMASK(h, l) \
295bcca84916cb5 Rikard Falkeborn 2020-04-06  38  	(GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
8bd9cb51daac893 Will Deacon      2018-06-19  39
kernel test robot Sept. 16, 2022, 1:04 a.m. UTC | #3 
Hi Lucas,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bb4f6b2281b11b009210f62eecd291f7b75c1e85]

url:    https://github.com/intel-lab-lkp/linux/commits/Lucas-De-Marchi/drm-i915-Improvements-to-stolen-memory-setup/20220916-044155
base:   bb4f6b2281b11b009210f62eecd291f7b75c1e85
config: i386-randconfig-a011 (https://download.01.org/0day-ci/archive/20220916/202209160835.MMBeObmU-lkp@intel.com/config)
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/b0c14c92efecabadc483675b606c1ce109cfa415
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Lucas-De-Marchi/drm-i915-Improvements-to-stolen-memory-setup/20220916-044155
        git checkout b0c14c92efecabadc483675b606c1ce109cfa415
        # 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 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>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/i915/gem/i915_gem_stolen.c:817:58: warning: shift count is negative [-Wshift-count-negative]
           dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & GEN12_BDSM_MASK;
                                                                   ^~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_reg.h:7956:28: note: expanded from macro 'GEN12_BDSM_MASK'
   #define   GEN12_BDSM_MASK               GENMASK(63, 20)
                                           ^~~~~~~~~~~~~~~
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^~~~~~~~~~~~~~~
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +817 drivers/gpu/drm/i915/gem/i915_gem_stolen.c

   798	
   799	struct intel_memory_region *
   800	i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
   801				   u16 instance)
   802	{
   803		struct intel_uncore *uncore = &i915->uncore;
   804		struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
   805		resource_size_t dsm_size, dsm_base, lmem_size;
   806		struct intel_memory_region *mem;
   807		resource_size_t io_start, io_size;
   808		resource_size_t min_page_size;
   809	
   810		if (WARN_ON_ONCE(instance))
   811			return ERR_PTR(-ENODEV);
   812	
   813		if (!i915_pci_resource_valid(pdev, GEN12_LMEM_BAR))
   814			return ERR_PTR(-ENXIO);
   815	
   816		/* Use DSM base address instead for stolen memory */
 > 817		dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & GEN12_BDSM_MASK;
   818		if (IS_DG1(uncore->i915)) {
   819			lmem_size = pci_resource_len(pdev, GEN12_LMEM_BAR);
   820			if (WARN_ON(lmem_size < dsm_base))
   821				return ERR_PTR(-ENODEV);
   822		} else {
   823			resource_size_t lmem_range;
   824	
   825			lmem_range = intel_gt_mcr_read_any(&i915->gt0, XEHP_TILE0_ADDR_RANGE) & 0xFFFF;
   826			lmem_size = lmem_range >> XEHP_TILE_LMEM_RANGE_SHIFT;
   827			lmem_size *= SZ_1G;
   828		}
   829	
   830		dsm_size = lmem_size - dsm_base;
   831		if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
   832			io_start = 0;
   833			io_size = 0;
   834		} else {
   835			io_start = pci_resource_start(pdev, GEN12_LMEM_BAR) + dsm_base;
   836			io_size = dsm_size;
   837		}
   838	
   839		min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
   840							I915_GTT_PAGE_SIZE_4K;
   841	
   842		mem = intel_memory_region_create(i915, dsm_base, dsm_size,
   843						 min_page_size,
   844						 io_start, io_size,
   845						 type, instance,
   846						 &i915_region_stolen_lmem_ops);
   847		if (IS_ERR(mem))
   848			return mem;
   849	
   850		/*
   851		 * TODO: consider creating common helper to just print all the
   852		 * interesting stuff from intel_memory_region, which we can use for all
   853		 * our probed regions.
   854		 */
   855	
   856		drm_dbg(&i915->drm, "Stolen Local memory IO start: %pa\n",
   857			&mem->io_start);
   858		drm_dbg(&i915->drm, "Stolen Local DSM base: %pa\n", &dsm_base);
   859	
   860		intel_memory_region_set_name(mem, "stolen-local");
   861	
   862		mem->private = true;
   863	
   864		return mem;
   865	}
   866
Iddamsetty, Aravind Sept. 16, 2022, 11:58 a.m. UTC | #4 
On 16-09-2022 02:09, Lucas De Marchi wrote:
> DSMBASE register is defined so BDSM bitfield contains the bits 63 to 20
> of the base address of stolen. For the supported platforms bits 0-19 are
> zero but that may not be true in future. Add the missing mask.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Acked-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>

Thanks,
Aravind.
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index 42f4769bb4ac..c34065fe2ecc 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -814,7 +814,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
>  		return ERR_PTR(-ENXIO);
>  
>  	/* Use DSM base address instead for stolen memory */
> -	dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE);
> +	dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & GEN12_BDSM_MASK;
>  	if (IS_DG1(uncore->i915)) {
>  		lmem_size = pci_resource_len(pdev, GEN12_LMEM_BAR);
>  		if (WARN_ON(lmem_size < dsm_base))
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1a9bd829fc7e..0301874c76ba 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7953,6 +7953,7 @@ enum skl_power_gate {
>  
>  #define GEN12_GSMBASE			_MMIO(0x108100)
>  #define GEN12_DSMBASE			_MMIO(0x1080C0)
> +#define   GEN12_BDSM_MASK		GENMASK(63, 20)
>  
>  #define XEHP_CLOCK_GATE_DIS		_MMIO(0x101014)
>  #define   SGSI_SIDECLK_DIS		REG_BIT(17)
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 42f4769bb4ac..c34065fe2ecc 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -814,7 +814,7 @@  i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
 		return ERR_PTR(-ENXIO);
 
 	/* Use DSM base address instead for stolen memory */
-	dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE);
+	dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & GEN12_BDSM_MASK;
 	if (IS_DG1(uncore->i915)) {
 		lmem_size = pci_resource_len(pdev, GEN12_LMEM_BAR);
 		if (WARN_ON(lmem_size < dsm_base))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1a9bd829fc7e..0301874c76ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7953,6 +7953,7 @@  enum skl_power_gate {
 
 #define GEN12_GSMBASE			_MMIO(0x108100)
 #define GEN12_DSMBASE			_MMIO(0x1080C0)
+#define   GEN12_BDSM_MASK		GENMASK(63, 20)
 
 #define XEHP_CLOCK_GATE_DIS		_MMIO(0x101014)
 #define   SGSI_SIDECLK_DIS		REG_BIT(17)