| Message ID | 20220906234934.3655440-5-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i915: Add "standalone media" support for MTL | expand |
On Tue, Sep 06, 2022 at 04:49:24PM -0700, Matt Roper wrote: >Unmapping of the MMIO range can be done as a DRM-managed action, which >will take care of the unmapping on device teardown and error paths. >This will also ensure proper ordering with respect to other DRM-managed >actions that we'll be using to clean up non-primary GTs in upcoming >patches. > >We have not yet enabled any non-root GTs in the driver yet, so the >kfree() of the GT structure is effectively dead code. When we do start >enabling non-root GTs in upcoming patches, those are going to be using >DRM-managed allocations tied to the device lifetime, so we don't need to >explicitly free them (and kfree would be incorrect anyway). > >Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Lucas De Marchi
Hi Matt,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on drm/drm-next drm-misc/drm-misc-next linus/master v6.0-rc4 next-20220906]
[cannot apply to drm-intel/for-linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Matt-Roper/i915-Add-standalone-media-support-for-MTL/20220907-075318
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-randconfig-s001
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/c06fd3ab7efd7036d4d9b61f4e8f2e585cc7771a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Matt-Roper/i915-Add-standalone-media-support-for-MTL/20220907-075318
git checkout c06fd3ab7efd7036d4d9b61f4e8f2e585cc7771a
# 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 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/i915/intel_uncore.c:2238:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got void *regs @@
drivers/gpu/drm/i915/intel_uncore.c:2238:17: sparse: expected void volatile [noderef] __iomem *addr
drivers/gpu/drm/i915/intel_uncore.c:2238:17: sparse: got void *regs
>> drivers/gpu/drm/i915/intel_uncore.c:2269:16: sparse: sparse: incorrect type in argument 3 (different address spaces) @@ expected void *data @@ got void [noderef] __iomem *regs @@
drivers/gpu/drm/i915/intel_uncore.c:2269:16: sparse: expected void *data
drivers/gpu/drm/i915/intel_uncore.c:2269:16: sparse: got void [noderef] __iomem *regs
drivers/gpu/drm/i915/intel_uncore.c:1843:1: sparse: sparse: context imbalance in 'fwtable_read8' - different lock contexts for basic block
drivers/gpu/drm/i915/intel_uncore.c:1844:1: sparse: sparse: context imbalance in 'fwtable_read16' - different lock contexts for basic block
drivers/gpu/drm/i915/intel_uncore.c:1845:1: sparse: sparse: context imbalance in 'fwtable_read32' - different lock contexts for basic block
drivers/gpu/drm/i915/intel_uncore.c:1846:1: sparse: sparse: context imbalance in 'fwtable_read64' - different lock contexts for basic block
drivers/gpu/drm/i915/intel_uncore.c:1909:1: sparse: sparse: context imbalance in 'gen6_write8' - different lock contexts for basic block
drivers/gpu/drm/i915/intel_uncore.c:1910:1: sparse: sparse: context imbalance in 'gen6_write16' - different lock contexts for basic block
drivers/gpu/drm/i915/intel_uncore.c:1911:1: sparse: sparse: context imbalance in 'gen6_write32' - different lock contexts for basic block
drivers/gpu/drm/i915/intel_uncore.c:1931:1: sparse: sparse: context imbalance in 'fwtable_write8' - different lock contexts for basic block
drivers/gpu/drm/i915/intel_uncore.c:1932:1: sparse: sparse: context imbalance in 'fwtable_write16' - different lock contexts for basic block
drivers/gpu/drm/i915/intel_uncore.c:1933:1: sparse: sparse: context imbalance in 'fwtable_write32' - different lock contexts for basic block
vim +2238 drivers/gpu/drm/i915/intel_uncore.c
2235
2236 static void uncore_unmap_mmio(struct drm_device *drm, void *regs)
2237 {
> 2238 iounmap(regs);
2239 }
2240
2241 int intel_uncore_setup_mmio(struct intel_uncore *uncore, phys_addr_t phys_addr)
2242 {
2243 struct drm_i915_private *i915 = uncore->i915;
2244 int mmio_size;
2245
2246 /*
2247 * Before gen4, the registers and the GTT are behind different BARs.
2248 * However, from gen4 onwards, the registers and the GTT are shared
2249 * in the same BAR, so we want to restrict this ioremap from
2250 * clobbering the GTT which we want ioremap_wc instead. Fortunately,
2251 * the register BAR remains the same size for all the earlier
2252 * generations up to Ironlake.
2253 * For dgfx chips register range is expanded to 4MB, and this larger
2254 * range is also used for integrated gpus beginning with Meteor Lake.
2255 */
2256 if (IS_DGFX(i915) || GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
2257 mmio_size = 4 * 1024 * 1024;
2258 else if (GRAPHICS_VER(i915) >= 5)
2259 mmio_size = 2 * 1024 * 1024;
2260 else
2261 mmio_size = 512 * 1024;
2262
2263 uncore->regs = ioremap(phys_addr, mmio_size);
2264 if (uncore->regs == NULL) {
2265 drm_err(&i915->drm, "failed to map registers\n");
2266 return -EIO;
2267 }
2268
> 2269 return drmm_add_action_or_reset(&i915->drm, uncore_unmap_mmio, uncore->regs);
2270 }
2271
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index cf7aab7adb30..663a4798fb2e 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -803,15 +803,6 @@ static int intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr) return 0; } -static void -intel_gt_tile_cleanup(struct intel_gt *gt) -{ - intel_uncore_cleanup_mmio(gt->uncore); - - if (!gt_is_root(gt)) - kfree(gt); -} - int intel_gt_probe_all(struct drm_i915_private *i915) { struct pci_dev *pdev = to_pci_dev(i915->drm.dev); @@ -858,10 +849,8 @@ void intel_gt_release_all(struct drm_i915_private *i915) struct intel_gt *gt; unsigned int id; - for_each_gt(gt, i915, id) { - intel_gt_tile_cleanup(gt); + for_each_gt(gt, i915, id) i915->gt[id] = NULL; - } } void intel_gt_info_print(const struct intel_gt_info *info, diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 6841f76533f9..2a32f8a65f34 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -21,6 +21,7 @@ * IN THE SOFTWARE. */ +#include <drm/drm_managed.h> #include <linux/pm_runtime.h> #include "gt/intel_engine_regs.h" @@ -2232,6 +2233,11 @@ static int i915_pmic_bus_access_notifier(struct notifier_block *nb, return NOTIFY_OK; } +static void uncore_unmap_mmio(struct drm_device *drm, void *regs) +{ + iounmap(regs); +} + int intel_uncore_setup_mmio(struct intel_uncore *uncore, phys_addr_t phys_addr) { struct drm_i915_private *i915 = uncore->i915; @@ -2260,12 +2266,7 @@ int intel_uncore_setup_mmio(struct intel_uncore *uncore, phys_addr_t phys_addr) return -EIO; } - return 0; -} - -void intel_uncore_cleanup_mmio(struct intel_uncore *uncore) -{ - iounmap(uncore->regs); + return drmm_add_action_or_reset(&i915->drm, uncore_unmap_mmio, uncore->regs); } void intel_uncore_init_early(struct intel_uncore *uncore,
Unmapping of the MMIO range can be done as a DRM-managed action, which will take care of the unmapping on device teardown and error paths. This will also ensure proper ordering with respect to other DRM-managed actions that we'll be using to clean up non-primary GTs in upcoming patches. We have not yet enabled any non-root GTs in the driver yet, so the kfree() of the GT structure is effectively dead code. When we do start enabling non-root GTs in upcoming patches, those are going to be using DRM-managed allocations tied to the device lifetime, so we don't need to explicitly free them (and kfree would be incorrect anyway). Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gt/intel_gt.c | 13 +------------ drivers/gpu/drm/i915/intel_uncore.c | 13 +++++++------ 2 files changed, 8 insertions(+), 18 deletions(-)