| Message ID | 1362768363-3710-9-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 08, 2013 at 10:45:52AM -0800, Jesse Barnes wrote: > The Gunit has a separate reg for this, so allocate some stolen space for > the power context and initialize the reg. > > v2: check for allocation before freeing at cleanup (Jani) > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_gem_stolen.c | 44 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > 3 files changed, 47 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 2b4d9b6..3061d73 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1023,6 +1023,8 @@ typedef struct drm_i915_private { > > struct i915_gpu_error gpu_error; > > + struct drm_mm_node *vlv_pctx; > + > /* list of fbdev register on this device */ > struct intel_fbdev *fbdev; > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index 69d97cb..8376a1b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -171,11 +171,52 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev) > dev_priv->cfb_size = 0; > } > > +static void i915_setup_pctx(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_mm_node *pctx; > + unsigned long pctx_paddr; > + int pctx_size = 24*1024; > + > + pctx = drm_mm_search_free(&dev_priv->mm.stolen, pctx_size, 4096, 0); > + if (pctx) > + pctx = drm_mm_get_block(pctx, pctx_size, 4096); > + if (!pctx) > + goto err; > + > + pctx_paddr = dev_priv->mm.stolen_base + pctx->start; > + if (!pctx_paddr) > + goto err_free_pctx; > + > + dev_priv->vlv_pctx = pctx; > + I915_WRITE(VLV_PCBR, pctx_paddr); > + > + return; > + > +err_free_pctx: > + drm_mm_put_block(pctx); > +err: > + DRM_DEBUG("not enough stolen space for PCTX, disabling\n"); > +} > + > +static void i915_cleanup_pctx(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (!dev_priv->vlv_pctx) > + return; > + > + I915_WRITE(VLV_PCBR, 0); > + drm_mm_put_block(dev_priv->vlv_pctx); > +} > + > void i915_gem_cleanup_stolen(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > i915_gem_stolen_cleanup_compression(dev); > + if (IS_VALLEYVIEW(dev) && i915_powersave) > + i915_cleanup_pctx(dev); > drm_mm_takedown(&dev_priv->mm.stolen); > } > > @@ -193,6 +234,9 @@ int i915_gem_init_stolen(struct drm_device *dev) > /* Basic memrange allocator for stolen space */ > drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size); > > + if (IS_VALLEYVIEW(dev) && i915_powersave) > + i915_setup_pctx(dev); > + > return 0; > } > These should happen in some valleyview init functions, no? Doing it with the stolen code seems sloppy to me. I'd also not mind either having a DRM_DEBUG_DRIVER msg, or some debugfs entry letting us know we're using power contexts. Equally an error state read of the register might be nice to have. > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c660a11..ed35805 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -623,6 +623,7 @@ > #define VLV_IIR (VLV_DISPLAY_BASE + 0x20a4) > #define VLV_IMR (VLV_DISPLAY_BASE + 0x20a8) > #define VLV_ISR (VLV_DISPLAY_BASE + 0x20ac) > +#define VLV_PCBR (VLV_DISPLAY_BASE + 0x2120) > #define I915_PIPE_CONTROL_NOTIFY_INTERRUPT (1<<18) > #define I915_DISPLAY_PORT_INTERRUPT (1<<17) > #define I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT (1<<15) > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Mar 08, 2013 at 10:45:52AM -0800, Jesse Barnes wrote: > The Gunit has a separate reg for this, so allocate some stolen space for > the power context and initialize the reg. Is it a requirement to use stolen space? The reason we're allocating rings and all from stolen is just to test things a bit better and ensure we have a decent chance that stolen handling for fastboot works ... Some comment somewhere would be good to clarify this. Also seconded on Ben's comment. Cheers, Daniel > > v2: check for allocation before freeing at cleanup (Jani) > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_gem_stolen.c | 44 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > 3 files changed, 47 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 2b4d9b6..3061d73 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1023,6 +1023,8 @@ typedef struct drm_i915_private { > > struct i915_gpu_error gpu_error; > > + struct drm_mm_node *vlv_pctx; > + > /* list of fbdev register on this device */ > struct intel_fbdev *fbdev; > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index 69d97cb..8376a1b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -171,11 +171,52 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev) > dev_priv->cfb_size = 0; > } > > +static void i915_setup_pctx(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_mm_node *pctx; > + unsigned long pctx_paddr; > + int pctx_size = 24*1024; > + > + pctx = drm_mm_search_free(&dev_priv->mm.stolen, pctx_size, 4096, 0); > + if (pctx) > + pctx = drm_mm_get_block(pctx, pctx_size, 4096); > + if (!pctx) > + goto err; > + > + pctx_paddr = dev_priv->mm.stolen_base + pctx->start; > + if (!pctx_paddr) > + goto err_free_pctx; > + > + dev_priv->vlv_pctx = pctx; > + I915_WRITE(VLV_PCBR, pctx_paddr); > + > + return; > + > +err_free_pctx: > + drm_mm_put_block(pctx); > +err: > + DRM_DEBUG("not enough stolen space for PCTX, disabling\n"); > +} > + > +static void i915_cleanup_pctx(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (!dev_priv->vlv_pctx) > + return; > + > + I915_WRITE(VLV_PCBR, 0); > + drm_mm_put_block(dev_priv->vlv_pctx); > +} > + > void i915_gem_cleanup_stolen(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > i915_gem_stolen_cleanup_compression(dev); > + if (IS_VALLEYVIEW(dev) && i915_powersave) > + i915_cleanup_pctx(dev); > drm_mm_takedown(&dev_priv->mm.stolen); > } > > @@ -193,6 +234,9 @@ int i915_gem_init_stolen(struct drm_device *dev) > /* Basic memrange allocator for stolen space */ > drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size); > > + if (IS_VALLEYVIEW(dev) && i915_powersave) > + i915_setup_pctx(dev); > + > return 0; > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c660a11..ed35805 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -623,6 +623,7 @@ > #define VLV_IIR (VLV_DISPLAY_BASE + 0x20a4) > #define VLV_IMR (VLV_DISPLAY_BASE + 0x20a8) > #define VLV_ISR (VLV_DISPLAY_BASE + 0x20ac) > +#define VLV_PCBR (VLV_DISPLAY_BASE + 0x2120) > #define I915_PIPE_CONTROL_NOTIFY_INTERRUPT (1<<18) > #define I915_DISPLAY_PORT_INTERRUPT (1<<17) > #define I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT (1<<15) > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Mar 12, 2013 at 12:40:30AM +0100, Daniel Vetter wrote: > On Fri, Mar 08, 2013 at 10:45:52AM -0800, Jesse Barnes wrote: > > The Gunit has a separate reg for this, so allocate some stolen space for > > the power context and initialize the reg. > > Is it a requirement to use stolen space? The reason we're allocating rings > and all from stolen is just to test things a bit better and ensure we have > a decent chance that stolen handling for fastboot works ... Some comment > somewhere would be good to clarify this. I'm in favour of making these small permanent allocations from stolen whereever possible. But you are right to point out that we need to clearly mark anthing that may require physically contiguous memory, just in case. -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2b4d9b6..3061d73 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1023,6 +1023,8 @@ typedef struct drm_i915_private { struct i915_gpu_error gpu_error; + struct drm_mm_node *vlv_pctx; + /* list of fbdev register on this device */ struct intel_fbdev *fbdev; diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 69d97cb..8376a1b 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -171,11 +171,52 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev) dev_priv->cfb_size = 0; } +static void i915_setup_pctx(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_mm_node *pctx; + unsigned long pctx_paddr; + int pctx_size = 24*1024; + + pctx = drm_mm_search_free(&dev_priv->mm.stolen, pctx_size, 4096, 0); + if (pctx) + pctx = drm_mm_get_block(pctx, pctx_size, 4096); + if (!pctx) + goto err; + + pctx_paddr = dev_priv->mm.stolen_base + pctx->start; + if (!pctx_paddr) + goto err_free_pctx; + + dev_priv->vlv_pctx = pctx; + I915_WRITE(VLV_PCBR, pctx_paddr); + + return; + +err_free_pctx: + drm_mm_put_block(pctx); +err: + DRM_DEBUG("not enough stolen space for PCTX, disabling\n"); +} + +static void i915_cleanup_pctx(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (!dev_priv->vlv_pctx) + return; + + I915_WRITE(VLV_PCBR, 0); + drm_mm_put_block(dev_priv->vlv_pctx); +} + void i915_gem_cleanup_stolen(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; i915_gem_stolen_cleanup_compression(dev); + if (IS_VALLEYVIEW(dev) && i915_powersave) + i915_cleanup_pctx(dev); drm_mm_takedown(&dev_priv->mm.stolen); } @@ -193,6 +234,9 @@ int i915_gem_init_stolen(struct drm_device *dev) /* Basic memrange allocator for stolen space */ drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size); + if (IS_VALLEYVIEW(dev) && i915_powersave) + i915_setup_pctx(dev); + return 0; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c660a11..ed35805 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -623,6 +623,7 @@ #define VLV_IIR (VLV_DISPLAY_BASE + 0x20a4) #define VLV_IMR (VLV_DISPLAY_BASE + 0x20a8) #define VLV_ISR (VLV_DISPLAY_BASE + 0x20ac) +#define VLV_PCBR (VLV_DISPLAY_BASE + 0x2120) #define I915_PIPE_CONTROL_NOTIFY_INTERRUPT (1<<18) #define I915_DISPLAY_PORT_INTERRUPT (1<<17) #define I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT (1<<15)
The Gunit has a separate reg for this, so allocate some stolen space for the power context and initialize the reg. v2: check for allocation before freeing at cleanup (Jani) Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_gem_stolen.c | 44 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 1 + 3 files changed, 47 insertions(+)