diff mbox series

[RFC,16/28] drm/i915: Compartmentalize i915_ggtt_probe_hw

Message ID 20190613133539.12620-17-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Implicit dev_priv removal and GT compartmentalization | expand

Commit Message

Tvrtko Ursulin June 13, 2019, 1:35 p.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Having made start to better code compartmentalization by introducing
struct intel_gt, continue the theme elsewhere in code by making functions
take parameters take what logically makes most sense for them instead of
the global struct drm_i915_private.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 31 +++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

Comments

Chris Wilson June 13, 2019, 2:03 p.m. UTC | #1 
Quoting Tvrtko Ursulin (2019-06-13 14:35:27)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Having made start to better code compartmentalization by introducing
> struct intel_gt, continue the theme elsewhere in code by making functions
> take parameters take what logically makes most sense for them instead of
> the global struct drm_i915_private.

Is that a can of worms I see? :)

While you are here, care to pull in the gmch probe so we can drop the
frankenstein approach.
-Chris
Tvrtko Ursulin June 14, 2019, 9:35 a.m. UTC | #2 
On 13/06/2019 15:03, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-06-13 14:35:27)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Having made start to better code compartmentalization by introducing
>> struct intel_gt, continue the theme elsewhere in code by making functions
>> take parameters take what logically makes most sense for them instead of
>> the global struct drm_i915_private.
> 
> Is that a can of worms I see? :)
> 
> While you are here, care to pull in the gmch probe so we can drop the
> frankenstein approach.

What exactly do you mean? Pull in what from where to where?

Regards,

Tvrtko
Chris Wilson June 14, 2019, 10:23 a.m. UTC | #3 
Quoting Tvrtko Ursulin (2019-06-14 10:35:57)
> 
> On 13/06/2019 15:03, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-06-13 14:35:27)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Having made start to better code compartmentalization by introducing
> >> struct intel_gt, continue the theme elsewhere in code by making functions
> >> take parameters take what logically makes most sense for them instead of
> >> the global struct drm_i915_private.
> > 
> > Is that a can of worms I see? :)
> > 
> > While you are here, care to pull in the gmch probe so we can drop the
> > frankenstein approach.
> 
> What exactly do you mean? Pull in what from where to where?

intel_gtt.ko is the other half of i915_gem_gtt.c
-Chris
Tvrtko Ursulin June 14, 2019, 3:01 p.m. UTC | #4 
On 14/06/2019 11:23, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-06-14 10:35:57)
>>
>> On 13/06/2019 15:03, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-06-13 14:35:27)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Having made start to better code compartmentalization by introducing
>>>> struct intel_gt, continue the theme elsewhere in code by making functions
>>>> take parameters take what logically makes most sense for them instead of
>>>> the global struct drm_i915_private.
>>>
>>> Is that a can of worms I see? :)
>>>
>>> While you are here, care to pull in the gmch probe so we can drop the
>>> frankenstein approach.
>>
>> What exactly do you mean? Pull in what from where to where?
> 
> intel_gtt.ko is the other half of i915_gem_gtt.c

I'll leave this out of this series, at least for now.

Otherwise I've done all the other refactors and tweaks to higher or 
lower standard. I'll send a new RFC out just so we see where we are.

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0810c1655224..c88213fa18af 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3507,21 +3507,16 @@  static int i915_gmch_probe(struct i915_ggtt *ggtt)
 	return 0;
 }
 
-/**
- * i915_ggtt_probe_hw - Probe GGTT hardware location
- * @dev_priv: i915 device
- */
-int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
+static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct drm_i915_private *i915)
 {
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	int ret;
 
-	ggtt->vm.i915 = dev_priv;
-	ggtt->vm.dma = &dev_priv->drm.pdev->dev;
+	ggtt->vm.i915 = i915;
+	ggtt->vm.dma = &i915->drm.pdev->dev;
 
-	if (INTEL_GEN(dev_priv) <= 5)
+	if (INTEL_GEN(i915) <= 5)
 		ret = i915_gmch_probe(ggtt);
-	else if (INTEL_GEN(dev_priv) < 8)
+	else if (INTEL_GEN(i915) < 8)
 		ret = gen6_gmch_probe(ggtt);
 	else
 		ret = gen8_gmch_probe(ggtt);
@@ -3549,6 +3544,22 @@  int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_DRIVER("GMADR size = %lluM\n", (u64)ggtt->mappable_end >> 20);
 	DRM_DEBUG_DRIVER("DSM size = %lluM\n",
 			 (u64)resource_size(&intel_graphics_stolen_res) >> 20);
+
+	return 0;
+}
+
+/**
+ * i915_ggtt_probe_hw - Probe GGTT hardware location
+ * @dev_priv: i915 device
+ */
+int i915_ggtt_probe_hw(struct drm_i915_private *i915)
+{
+	int ret;
+
+	ret = ggtt_probe_hw(&i915->ggtt, i915);
+	if (ret)
+		return ret;
+
 	if (intel_vtd_active())
 		DRM_INFO("VT-d active for gfx access\n");