Re: [Intel-gfx] [PATCH v2] drm/i915: Refactor setting dma info to a common helper

2020-04-17 Thread Ruhl, Michael J


>-Original Message-
>From: Chris Wilson 
>Sent: Friday, April 17, 2020 3:55 PM
>To: Ruhl, Michael J ; intel-
>g...@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Refactor setting dma info to a
>common helper
>
>Quoting Michael J. Ruhl (2020-04-17 20:51:07)
>> DMA_MASK bit values are different for different generations.
>>
>> This will become more difficult to manage over time with the open
>> coded usage of different versions of the device.
>>
>> Fix by:
>>   disallow setting of dma mask in AGP path (< GEN(5) for i915,
>>   add dma_mask_size to the device info configuration,
>>   updating open code call sequence to the latest interface,
>>   refactoring into a common function for setting the dma segment
>>   and mask info
>>
>> Reviewed-by: Chris Wilson 
>> Signed-off-by: Michael J. Ruhl 
>> cc: Brian Welty 
>> cc: Daniele Ceraolo Spurio 
>>
>> ---
>> v1: removed i915 depenancy from agp path for dma mask
>> Consolidated segment size and work arounds to the helper
>> v2: added r-b
>
>You don't need to resend for adding r-b by itself, patchwork will do
>that, and the committer should be checking the output from pw. dim then
>double checks that we haven't missed anything vital.

Cool.

I will refrain from future sends. 😊

M

>-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] drm/i915: Refactor setting dma info to a common helper

2020-04-17 Thread Chris Wilson
Quoting Michael J. Ruhl (2020-04-17 20:51:07)
> DMA_MASK bit values are different for different generations.
> 
> This will become more difficult to manage over time with the open
> coded usage of different versions of the device.
> 
> Fix by:
>   disallow setting of dma mask in AGP path (< GEN(5) for i915,
>   add dma_mask_size to the device info configuration,
>   updating open code call sequence to the latest interface,
>   refactoring into a common function for setting the dma segment
>   and mask info
> 
> Reviewed-by: Chris Wilson 
> Signed-off-by: Michael J. Ruhl 
> cc: Brian Welty 
> cc: Daniele Ceraolo Spurio 
> 
> ---
> v1: removed i915 depenancy from agp path for dma mask
> Consolidated segment size and work arounds to the helper
> v2: added r-b

You don't need to resend for adding r-b by itself, patchwork will do
that, and the committer should be checking the output from pw. dim then
double checks that we haven't missed anything vital.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2] drm/i915: Refactor setting dma info to a common helper

2020-04-17 Thread Michael J. Ruhl
DMA_MASK bit values are different for different generations.

This will become more difficult to manage over time with the open
coded usage of different versions of the device.

Fix by:
  disallow setting of dma mask in AGP path (< GEN(5) for i915,
  add dma_mask_size to the device info configuration,
  updating open code call sequence to the latest interface,
  refactoring into a common function for setting the dma segment
  and mask info

Reviewed-by: Chris Wilson 
Signed-off-by: Michael J. Ruhl 
cc: Brian Welty 
cc: Daniele Ceraolo Spurio 

---
v1: removed i915 depenancy from agp path for dma mask
Consolidated segment size and work arounds to the helper
v2: added r-b

---
 drivers/char/agp/intel-gtt.c | 17 +++--
 drivers/gpu/drm/i915/gt/intel_ggtt.c | 15 
 drivers/gpu/drm/i915/i915_drv.c  | 94 +++-
 drivers/gpu/drm/i915/i915_pci.c  | 14 
 drivers/gpu/drm/i915/intel_device_info.c |  1 +
 drivers/gpu/drm/i915/intel_device_info.h |  2 +
 6 files changed, 87 insertions(+), 56 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 3d42fc4290bc..4b34a5195c65 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1407,13 +1407,16 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, 
struct pci_dev *gpu_pdev,
 
dev_info(&bridge_pdev->dev, "Intel %s Chipset\n", 
intel_gtt_chipsets[i].name);
 
-   mask = intel_private.driver->dma_mask_size;
-   if (pci_set_dma_mask(intel_private.pcidev, DMA_BIT_MASK(mask)))
-   dev_err(&intel_private.pcidev->dev,
-   "set gfx device dma mask %d-bit failed!\n", mask);
-   else
-   pci_set_consistent_dma_mask(intel_private.pcidev,
-   DMA_BIT_MASK(mask));
+   if (bridge) {
+   mask = intel_private.driver->dma_mask_size;
+   if (pci_set_dma_mask(intel_private.pcidev, DMA_BIT_MASK(mask)))
+   dev_err(&intel_private.pcidev->dev,
+   "set gfx device dma mask %d-bit failed!\n",
+   mask);
+   else
+   pci_set_consistent_dma_mask(intel_private.pcidev,
+   DMA_BIT_MASK(mask));
+   }
 
if (intel_gtt_init() != 0) {
intel_gmch_remove();
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index eebd1190506f..66165b10256e 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -840,7 +840,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
struct pci_dev *pdev = i915->drm.pdev;
unsigned int size;
u16 snb_gmch_ctl;
-   int err;
 
/* TODO: We're not aware of mappable constraints on gen8 yet */
if (!IS_DGFX(i915)) {
@@ -848,13 +847,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
ggtt->mappable_end = resource_size(&ggtt->gmadr);
}
 
-   err = pci_set_dma_mask(pdev, DMA_BIT_MASK(39));
-   if (!err)
-   err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(39));
-   if (err)
-   drm_err(&i915->drm,
-   "Can't set DMA mask/consistent mask (%d)\n", err);
-
pci_read_config_word(pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
if (IS_CHERRYVIEW(i915))
size = chv_get_total_gtt_size(snb_gmch_ctl);
@@ -990,7 +982,6 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
struct pci_dev *pdev = i915->drm.pdev;
unsigned int size;
u16 snb_gmch_ctl;
-   int err;
 
ggtt->gmadr = pci_resource(pdev, 2);
ggtt->mappable_end = resource_size(&ggtt->gmadr);
@@ -1005,12 +996,6 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
return -ENXIO;
}
 
-   err = pci_set_dma_mask(pdev, DMA_BIT_MASK(40));
-   if (!err)
-   err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(40));
-   if (err)
-   drm_err(&i915->drm,
-   "Can't set DMA mask/consistent mask (%d)\n", err);
pci_read_config_word(pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
 
size = gen6_get_total_gtt_size(snb_gmch_ctl);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 641f5e03b661..21e30b30d9d6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -566,6 +566,62 @@ static void intel_sanitize_options(struct drm_i915_private 
*dev_priv)
intel_gvt_sanitize_options(dev_priv);
 }
 
+/**
+ * i915_set_dma_info - set all relevant PCI dma info as configured for the
+ * platform
+ * @i915: valid i915 instance
+ *
+ * Set the dma max segment size, device and coherent masks.  The dma mask set
+ * needs to occur before i915_ggtt_probe_hw.
+ *
+ * A couple of platforms have special needs.  Address them as well.