Re: [PATCH 1/7] drm/i915: add simple wrappers for stolen node insertion/removal

Chris Wilson
2015-07-02T13:36:18Z

On Thu, Jul 02, 2015 at 10:33:27AM -0300, Paulo Zanoni wrote:
> 2015-07-01 17:38 GMT-03:00 Chris Wilson <chris@chris-wilson.co.uk>:
> > On Wed, Jul 01, 2015 at 05:15:20PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> We want to move the FBC code out of i915_gem_stolen.c, but that code
> >> directly adds/removes stolen memory nodes. Let's create this
> >> abstraction, so i915_gme_stolen.c is still in control of all the
> >> stolen memory handling. These abstractions will also allow us to add
> >> locking assertions later.
> >>
> >> Requested-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.h | 4 ++++
> >> drivers/gpu/drm/i915/i915_gem_stolen.c | 44 +++++++++++++++++++++-------------
> >> 2 files changed, 32 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 1dbd957..b9de374 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -3109,6 +3109,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
> >> }
> >>
> >> /* i915_gem_stolen.c */
> >> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> >> + struct drm_mm_node *node, u64 size,
> >> + unsigned alignment);
> >> +void i915_gem_stolen_remove_node(struct drm_mm_node *node);
> >
> > Might as well pass in dev_priv now to save changing the interface later.
>
> I thought about doing it, but dev_priv is just for the mutex WARN, and
> I thought it could be rejected, so I didn't add dev_priv because of
> the risk of having the WARN rejected, so dev_priv would be useless.
> But I can change this, no problem.

If you change it over to taking the stolen_mutex inside these functions
(which is the only place we need to), then it is required.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre