Re: [PATCH 0/5] drm/i915: cleanup dead code
On Tue, Mar 12, 2024 at 09:54:41AM +, Tvrtko Ursulin wrote: On 11/03/2024 19:27, Lucas De Marchi wrote: On Mon, Mar 11, 2024 at 05:43:00PM +, Tvrtko Ursulin wrote: On 06/03/2024 19:36, Lucas De Marchi wrote: Remove platforms that never had their PCI IDs added to the driver and are of course marked with requiring force_probe. Note that most of the code for those platforms is actually used by subsequent ones, so it's not a huge amount of code being removed. I had PVC and xehpsdv back in October but could not collect all acks. :( Last two patches from https://patchwork.freedesktop.org/series/124705/. oh... I was actually surprised we still had xehpsdv while removing a WA for PVC, which made me look into removing these platforms. rebasing your series and comparing yours..my-v2, where my-v2 only has patches 2 and 4, I have the diff below. I think it's small enough that I can just take your commits and squash delta. Is that ok to you? my version is a little bit more aggressive, also doing some renames s/xehpsdv/xehp/ and dropping some more code (engine_mask_apply_copy_fuses(), unused registers, default ctx, fw ranges). Right, yeah I see I missed some case combos in the comments when grepping and more. diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h index 8a8fcd4fceac..bc26dc126104 100644 --- a/Documentation/gpu/rfc/i915_vm_bind.h +++ b/Documentation/gpu/rfc/i915_vm_bind.h @@ -93,12 +93,11 @@ struct drm_i915_gem_timeline_fence { * Multiple VA mappings can be created to the same section of the object * (aliasing). * - * The @start, @offset and @length must be 4K page aligned. However the DG2 - * and XEHPSDV has 64K page size for device local memory and has compact page - * table. On those platforms, for binding device local-memory objects, the - * @start, @offset and @length must be 64K aligned. Also, UMDs should not mix - * the local memory 64K page and the system memory 4K page bindings in the same - * 2M range. + * The @start, @offset and @length must be 4K page aligned. However the DG2 has + * 64K page size for device local memory and has compact page table. On that + * platform, for binding device local-memory objects, the @start, @offset and + * @length must be 64K aligned. Also, UMDs should not mix the local memory 64K + * page and the system memory 4K page bindings in the same 2M range. * * Error code -EINVAL will be returned if @start, @offset and @length are not * properly aligned. In version 1 (See I915_PARAM_VM_BIND_VERSION), error code diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 1495b6074492..d3300ae3053f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -386,7 +386,7 @@ struct drm_i915_gem_object { * and kernel mode driver for caching policy control after GEN12. * In the meantime platform specific tables are created to translate * i915_cache_level into pat index, for more details check the macros - * defined i915/i915_pci.c, e.g. TGL_CACHELEVEL. + * defined i915/i915_pci.c, e.g. MTL_CACHELEVEL. Why this? it was just our different choices while doing the search-and-replace. It's not that I changed yours, it's that my choice was to go with MTL and yours to go with TGL. Any of them fit the role here. * For backward compatibility, this field contains values exactly match * the entries of enum i915_cache_level for pre-GEN12 platforms (See * LEGACY_CACHELEVEL), so that the PTE encode functions for these diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index fa46d2308b0e..1bd0e041e15c 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -500,11 +500,11 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, } static void -xehpsdv_ppgtt_insert_huge(struct i915_address_space *vm, - struct i915_vma_resource *vma_res, - struct sgt_dma *iter, - unsigned int pat_index, - u32 flags) +xehp_ppgtt_insert_huge(struct i915_address_space *vm, + struct i915_vma_resource *vma_res, + struct sgt_dma *iter, + unsigned int pat_index, + u32 flags) { const gen8_pte_t pte_encode = vm->pte_encode(0, pat_index, flags); unsigned int rem = sg_dma_len(iter->sg); @@ -741,8 +741,8 @@ static void gen8_ppgtt_insert(struct i915_address_space *vm, struct sgt_dma iter = sgt_dma(vma_res); if (vma_res->bi.page_sizes.sg > I915_GTT_PAGE_SIZE) { - if (GRAPHICS_VER_FULL(vm->i915) >= IP_VER(12, 50)) -
Re: [PATCH 0/5] drm/i915: cleanup dead code
On 11/03/2024 19:27, Lucas De Marchi wrote: On Mon, Mar 11, 2024 at 05:43:00PM +, Tvrtko Ursulin wrote: On 06/03/2024 19:36, Lucas De Marchi wrote: Remove platforms that never had their PCI IDs added to the driver and are of course marked with requiring force_probe. Note that most of the code for those platforms is actually used by subsequent ones, so it's not a huge amount of code being removed. I had PVC and xehpsdv back in October but could not collect all acks. :( Last two patches from https://patchwork.freedesktop.org/series/124705/. oh... I was actually surprised we still had xehpsdv while removing a WA for PVC, which made me look into removing these platforms. rebasing your series and comparing yours..my-v2, where my-v2 only has patches 2 and 4, I have the diff below. I think it's small enough that I can just take your commits and squash delta. Is that ok to you? my version is a little bit more aggressive, also doing some renames s/xehpsdv/xehp/ and dropping some more code (engine_mask_apply_copy_fuses(), unused registers, default ctx, fw ranges). Right, yeah I see I missed some case combos in the comments when grepping and more. diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h index 8a8fcd4fceac..bc26dc126104 100644 --- a/Documentation/gpu/rfc/i915_vm_bind.h +++ b/Documentation/gpu/rfc/i915_vm_bind.h @@ -93,12 +93,11 @@ struct drm_i915_gem_timeline_fence { * Multiple VA mappings can be created to the same section of the object * (aliasing). * - * The @start, @offset and @length must be 4K page aligned. However the DG2 - * and XEHPSDV has 64K page size for device local memory and has compact page - * table. On those platforms, for binding device local-memory objects, the - * @start, @offset and @length must be 64K aligned. Also, UMDs should not mix - * the local memory 64K page and the system memory 4K page bindings in the same - * 2M range. + * The @start, @offset and @length must be 4K page aligned. However the DG2 has + * 64K page size for device local memory and has compact page table. On that + * platform, for binding device local-memory objects, the @start, @offset and + * @length must be 64K aligned. Also, UMDs should not mix the local memory 64K + * page and the system memory 4K page bindings in the same 2M range. * * Error code -EINVAL will be returned if @start, @offset and @length are not * properly aligned. In version 1 (See I915_PARAM_VM_BIND_VERSION), error code diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 1495b6074492..d3300ae3053f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -386,7 +386,7 @@ struct drm_i915_gem_object { * and kernel mode driver for caching policy control after GEN12. * In the meantime platform specific tables are created to translate * i915_cache_level into pat index, for more details check the macros - * defined i915/i915_pci.c, e.g. TGL_CACHELEVEL. + * defined i915/i915_pci.c, e.g. MTL_CACHELEVEL. Why this? * For backward compatibility, this field contains values exactly match * the entries of enum i915_cache_level for pre-GEN12 platforms (See * LEGACY_CACHELEVEL), so that the PTE encode functions for these diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index fa46d2308b0e..1bd0e041e15c 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -500,11 +500,11 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, } static void -xehpsdv_ppgtt_insert_huge(struct i915_address_space *vm, - struct i915_vma_resource *vma_res, - struct sgt_dma *iter, - unsigned int pat_index, - u32 flags) +xehp_ppgtt_insert_huge(struct i915_address_space *vm, + struct i915_vma_resource *vma_res, + struct sgt_dma *iter, + unsigned int pat_index, + u32 flags) { const gen8_pte_t pte_encode = vm->pte_encode(0, pat_index, flags); unsigned int rem = sg_dma_len(iter->sg); @@ -741,8 +741,8 @@ static void gen8_ppgtt_insert(struct i915_address_space *vm, struct sgt_dma iter = sgt_dma(vma_res); if (vma_res->bi.page_sizes.sg > I915_GTT_PAGE_SIZE) { - if (GRAPHICS_VER_FULL(vm->i915) >= IP_VER(12, 50)) - xehpsdv_ppgtt_insert_huge(vm, vma_res, &iter, pat_index, flags); + if (GRAPHICS_VER_FULL(vm->i915) >= IP_VER(12, 55)) + xehp_ppgtt_insert_huge(vm, vma_res, &iter, pat_index,
Re: [PATCH 0/5] drm/i915: cleanup dead code
On Mon, Mar 11, 2024 at 05:43:00PM +, Tvrtko Ursulin wrote: On 06/03/2024 19:36, Lucas De Marchi wrote: Remove platforms that never had their PCI IDs added to the driver and are of course marked with requiring force_probe. Note that most of the code for those platforms is actually used by subsequent ones, so it's not a huge amount of code being removed. I had PVC and xehpsdv back in October but could not collect all acks. :( Last two patches from https://patchwork.freedesktop.org/series/124705/. oh... I was actually surprised we still had xehpsdv while removing a WA for PVC, which made me look into removing these platforms. rebasing your series and comparing yours..my-v2, where my-v2 only has patches 2 and 4, I have the diff below. I think it's small enough that I can just take your commits and squash delta. Is that ok to you? my version is a little bit more aggressive, also doing some renames s/xehpsdv/xehp/ and dropping some more code (engine_mask_apply_copy_fuses(), unused registers, default ctx, fw ranges). diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h index 8a8fcd4fceac..bc26dc126104 100644 --- a/Documentation/gpu/rfc/i915_vm_bind.h +++ b/Documentation/gpu/rfc/i915_vm_bind.h @@ -93,12 +93,11 @@ struct drm_i915_gem_timeline_fence { * Multiple VA mappings can be created to the same section of the object * (aliasing). * - * The @start, @offset and @length must be 4K page aligned. However the DG2 - * and XEHPSDV has 64K page size for device local memory and has compact page - * table. On those platforms, for binding device local-memory objects, the - * @start, @offset and @length must be 64K aligned. Also, UMDs should not mix - * the local memory 64K page and the system memory 4K page bindings in the same - * 2M range. + * The @start, @offset and @length must be 4K page aligned. However the DG2 has + * 64K page size for device local memory and has compact page table. On that + * platform, for binding device local-memory objects, the @start, @offset and + * @length must be 64K aligned. Also, UMDs should not mix the local memory 64K + * page and the system memory 4K page bindings in the same 2M range. * * Error code -EINVAL will be returned if @start, @offset and @length are not * properly aligned. In version 1 (See I915_PARAM_VM_BIND_VERSION), error code diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 1495b6074492..d3300ae3053f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -386,7 +386,7 @@ struct drm_i915_gem_object { * and kernel mode driver for caching policy control after GEN12. * In the meantime platform specific tables are created to translate * i915_cache_level into pat index, for more details check the macros -* defined i915/i915_pci.c, e.g. TGL_CACHELEVEL. +* defined i915/i915_pci.c, e.g. MTL_CACHELEVEL. * For backward compatibility, this field contains values exactly match * the entries of enum i915_cache_level for pre-GEN12 platforms (See * LEGACY_CACHELEVEL), so that the PTE encode functions for these diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index fa46d2308b0e..1bd0e041e15c 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -500,11 +500,11 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, } static void -xehpsdv_ppgtt_insert_huge(struct i915_address_space *vm, - struct i915_vma_resource *vma_res, - struct sgt_dma *iter, - unsigned int pat_index, - u32 flags) +xehp_ppgtt_insert_huge(struct i915_address_space *vm, + struct i915_vma_resource *vma_res, + struct sgt_dma *iter, + unsigned int pat_index, + u32 flags) { const gen8_pte_t pte_encode = vm->pte_encode(0, pat_index, flags); unsigned int rem = sg_dma_len(iter->sg); @@ -741,8 +741,8 @@ static void gen8_ppgtt_insert(struct i915_address_space *vm, struct sgt_dma iter = sgt_dma(vma_res); if (vma_res->bi.page_sizes.sg > I915_GTT_PAGE_SIZE) { - if (GRAPHICS_VER_FULL(vm->i915) >= IP_VER(12, 50)) - xehpsdv_ppgtt_insert_huge(vm, vma_res, &iter, pat_ind
Re: [PATCH 0/5] drm/i915: cleanup dead code
On 06/03/2024 19:36, Lucas De Marchi wrote: Remove platforms that never had their PCI IDs added to the driver and are of course marked with requiring force_probe. Note that most of the code for those platforms is actually used by subsequent ones, so it's not a huge amount of code being removed. I had PVC and xehpsdv back in October but could not collect all acks. :( Last two patches from https://patchwork.freedesktop.org/series/124705/. Regards, Tvrtko drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h is also changed on the xe side, but that should be ok: the defines are there only for compat reasons while building the display side (and none of these platforms have display, so it's build-issue only). First patch is what motivated the others and was submitted alone @ 20240306144723.1826977-1-lucas.demar...@intel.com . While loooking at this WA I was wondering why we still had some of that code around. Build-tested only for now. Lucas De Marchi (5): drm/i915: Drop WA 16015675438 drm/i915: Drop dead code for xehpsdv drm/i915: Update IP_VER(12, 50) drm/i915: Drop dead code for pvc drm/i915: Remove special handling for !RCS_MASK() Documentation/gpu/rfc/i915_vm_bind.h | 11 +- .../gpu/drm/i915/gem/i915_gem_object_types.h | 2 +- .../gpu/drm/i915/gem/selftests/huge_pages.c | 4 +- .../i915/gem/selftests/i915_gem_client_blt.c | 8 +- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 5 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 40 ++-- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 43 +--- .../drm/i915/gt/intel_execlists_submission.c | 10 +- drivers/gpu/drm/i915/gt/intel_gsc.c | 15 -- drivers/gpu/drm/i915/gt/intel_gt.c| 4 +- drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 52 + drivers/gpu/drm/i915/gt/intel_gt_mcr.h| 2 +- drivers/gpu/drm/i915/gt/intel_gt_regs.h | 59 -- drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 21 +- drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 51 + drivers/gpu/drm/i915/gt/intel_migrate.c | 22 +- drivers/gpu/drm/i915/gt/intel_mocs.c | 52 + drivers/gpu/drm/i915/gt/intel_rps.c | 6 +- drivers/gpu/drm/i915/gt/intel_sseu.c | 13 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 193 +- drivers/gpu/drm/i915/gt/uc/intel_guc.c| 6 +- drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 4 +- drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 1 - .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- drivers/gpu/drm/i915/gt/uc/intel_uc.c | 4 - drivers/gpu/drm/i915/i915_debugfs.c | 12 -- drivers/gpu/drm/i915/i915_drv.h | 13 -- drivers/gpu/drm/i915/i915_getparam.c | 4 +- drivers/gpu/drm/i915/i915_gpu_error.c | 5 +- drivers/gpu/drm/i915/i915_hwmon.c | 6 - drivers/gpu/drm/i915/i915_pci.c | 61 +- drivers/gpu/drm/i915/i915_perf.c | 19 +- drivers/gpu/drm/i915/i915_query.c | 2 +- drivers/gpu/drm/i915/i915_reg.h | 4 +- drivers/gpu/drm/i915/intel_clock_gating.c | 26 +-- drivers/gpu/drm/i915/intel_device_info.c | 2 - drivers/gpu/drm/i915/intel_device_info.h | 2 - drivers/gpu/drm/i915/intel_step.c | 80 +--- drivers/gpu/drm/i915/intel_uncore.c | 159 +-- drivers/gpu/drm/i915/selftests/intel_uncore.c | 3 - .../gpu/drm/xe/compat-i915-headers/i915_drv.h | 6 - 43 files changed, 110 insertions(+), 928 deletions(-)