* [PATCH 01/10] drm/i915: No LLC_MLC for HSW.
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-23 10:15 ` Mika Kuoppala
2012-10-25 20:47 ` Jesse Barnes
2012-10-23 1:34 ` [PATCH 02/10] drm/i915: Add dev to ppgtt Ben Widawsky
` (9 subsequent siblings)
10 siblings, 2 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
The mid-level cache or as it's more commonly referred to now as L3, is
not setup this way on HSW.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 47e427e..5751ad2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -218,7 +218,11 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
switch (cache_level) {
case I915_CACHE_LLC_MLC:
- pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
+ /* Haswell doesn't set L3 this way */
+ if (IS_HASWELL(obj->base.dev))
+ pte_flags |= GEN6_PTE_CACHE_LLC;
+ else
+ pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
break;
case I915_CACHE_LLC:
pte_flags |= GEN6_PTE_CACHE_LLC;
@@ -253,12 +257,12 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev,
{
switch (cache_level) {
case I915_CACHE_LLC_MLC:
- if (INTEL_INFO(dev)->gen >= 6)
- return AGP_USER_CACHED_MEMORY_LLC_MLC;
/* Older chipsets do not have this extra level of CPU
* cacheing, so fallthrough and request the PTE simply
* as cached.
*/
+ if (INTEL_INFO(dev)->gen >= 6 && !IS_HASWELL(dev))
+ return AGP_USER_CACHED_MEMORY_LLC_MLC;
case I915_CACHE_LLC:
return AGP_USER_CACHED_MEMORY;
default:
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 01/10] drm/i915: No LLC_MLC for HSW.
2012-10-23 1:34 ` [PATCH 01/10] drm/i915: No LLC_MLC for HSW Ben Widawsky
@ 2012-10-23 10:15 ` Mika Kuoppala
2012-10-25 20:47 ` Jesse Barnes
1 sibling, 0 replies; 50+ messages in thread
From: Mika Kuoppala @ 2012-10-23 10:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Mon, 22 Oct 2012 18:34:06 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> The mid-level cache or as it's more commonly referred to now as L3, is
> not setup this way on HSW.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 47e427e..5751ad2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -218,7 +218,11 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
>
> switch (cache_level) {
> case I915_CACHE_LLC_MLC:
> - pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> + /* Haswell doesn't set L3 this way */
> + if (IS_HASWELL(obj->base.dev))
> + pte_flags |= GEN6_PTE_CACHE_LLC;
> + else
> + pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> break;
> case I915_CACHE_LLC:
> pte_flags |= GEN6_PTE_CACHE_LLC;
> @@ -253,12 +257,12 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> {
> switch (cache_level) {
> case I915_CACHE_LLC_MLC:
> - if (INTEL_INFO(dev)->gen >= 6)
> - return AGP_USER_CACHED_MEMORY_LLC_MLC;
> /* Older chipsets do not have this extra level of CPU
> * cacheing, so fallthrough and request the PTE simply
> * as cached.
> */
> + if (INTEL_INFO(dev)->gen >= 6 && !IS_HASWELL(dev))
> + return AGP_USER_CACHED_MEMORY_LLC_MLC;
> case I915_CACHE_LLC:
> return AGP_USER_CACHED_MEMORY;
> default:
> --
> 1.7.12.4
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 01/10] drm/i915: No LLC_MLC for HSW.
2012-10-23 1:34 ` [PATCH 01/10] drm/i915: No LLC_MLC for HSW Ben Widawsky
2012-10-23 10:15 ` Mika Kuoppala
@ 2012-10-25 20:47 ` Jesse Barnes
2012-10-26 1:03 ` Ben Widawsky
1 sibling, 1 reply; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 20:47 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:06 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> The mid-level cache or as it's more commonly referred to now as L3, is
> not setup this way on HSW.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 47e427e..5751ad2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -218,7 +218,11 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
>
> switch (cache_level) {
> case I915_CACHE_LLC_MLC:
> - pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> + /* Haswell doesn't set L3 this way */
> + if (IS_HASWELL(obj->base.dev))
> + pte_flags |= GEN6_PTE_CACHE_LLC;
> + else
> + pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> break;
> case I915_CACHE_LLC:
> pte_flags |= GEN6_PTE_CACHE_LLC;
> @@ -253,12 +257,12 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> {
> switch (cache_level) {
> case I915_CACHE_LLC_MLC:
> - if (INTEL_INFO(dev)->gen >= 6)
> - return AGP_USER_CACHED_MEMORY_LLC_MLC;
> /* Older chipsets do not have this extra level of CPU
> * cacheing, so fallthrough and request the PTE simply
> * as cached.
> */
> + if (INTEL_INFO(dev)->gen >= 6 && !IS_HASWELL(dev))
> + return AGP_USER_CACHED_MEMORY_LLC_MLC;
> case I915_CACHE_LLC:
> return AGP_USER_CACHED_MEMORY;
> default:
We might want a .has_mlc or something here at some point, but that
doesn't have to happen here.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 01/10] drm/i915: No LLC_MLC for HSW.
2012-10-25 20:47 ` Jesse Barnes
@ 2012-10-26 1:03 ` Ben Widawsky
0 siblings, 0 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-26 1:03 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Thu, 25 Oct 2012 13:47:22 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 22 Oct 2012 18:34:06 -0700
> Ben Widawsky <ben@bwidawsk.net> wrote:
>
> > The mid-level cache or as it's more commonly referred to now as L3,
> > is not setup this way on HSW.
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c index 47e427e..5751ad2 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -218,7 +218,11 @@ void i915_ppgtt_bind_object(struct
> > i915_hw_ppgtt *ppgtt,
> > switch (cache_level) {
> > case I915_CACHE_LLC_MLC:
> > - pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> > + /* Haswell doesn't set L3 this way */
> > + if (IS_HASWELL(obj->base.dev))
> > + pte_flags |= GEN6_PTE_CACHE_LLC;
> > + else
> > + pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> > break;
> > case I915_CACHE_LLC:
> > pte_flags |= GEN6_PTE_CACHE_LLC;
> > @@ -253,12 +257,12 @@ static unsigned int
> > cache_level_to_agp_type(struct drm_device *dev, {
> > switch (cache_level) {
> > case I915_CACHE_LLC_MLC:
> > - if (INTEL_INFO(dev)->gen >= 6)
> > - return AGP_USER_CACHED_MEMORY_LLC_MLC;
> > /* Older chipsets do not have this extra level of
> > CPU
> > * cacheing, so fallthrough and request the PTE
> > simply
> > * as cached.
> > */
> > + if (INTEL_INFO(dev)->gen >= 6 && !IS_HASWELL(dev))
> > + return AGP_USER_CACHED_MEMORY_LLC_MLC;
> > case I915_CACHE_LLC:
> > return AGP_USER_CACHED_MEMORY;
> > default:
>
> We might want a .has_mlc or something here at some point, but that
> doesn't have to happen here.
>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>
To be clear, HSW has MLC, (and I'd vote we converge on calling calling
it GPU L3 or something since docs don't seem to use the term MLC
anymore) it just doesn't get set in the PTE anymore.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 02/10] drm/i915: Add dev to ppgtt
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
2012-10-23 1:34 ` [PATCH 01/10] drm/i915: No LLC_MLC for HSW Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-25 20:48 ` Jesse Barnes
2012-10-23 1:34 ` [PATCH 03/10] drm/i915: introduce gtt_pte_t Ben Widawsky
` (8 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Some subsequent commits will need to know what generation we're running
on to do different pte encoding for the ppgtt. Since it's not much
hassle or overhead to store it in the ppgtt structure, do that.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index af0e97e..bf628c4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -344,6 +344,7 @@ struct intel_device_info {
#define I915_PPGTT_PD_ENTRIES 512
#define I915_PPGTT_PT_ENTRIES 1024
struct i915_hw_ppgtt {
+ struct drm_device *dev;
unsigned num_pd_entries;
struct page **pt_pages;
uint32_t pd_offset;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5751ad2..2b75028 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -78,6 +78,7 @@ int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
if (!ppgtt)
return ret;
+ ppgtt->dev = dev;
ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES;
ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
GFP_KERNEL);
@@ -219,7 +220,7 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
switch (cache_level) {
case I915_CACHE_LLC_MLC:
/* Haswell doesn't set L3 this way */
- if (IS_HASWELL(obj->base.dev))
+ if (IS_HASWELL(ppgtt->dev))
pte_flags |= GEN6_PTE_CACHE_LLC;
else
pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
@@ -228,7 +229,7 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
pte_flags |= GEN6_PTE_CACHE_LLC;
break;
case I915_CACHE_NONE:
- if (IS_HASWELL(obj->base.dev))
+ if (IS_HASWELL(ppgtt->dev))
pte_flags |= HSW_PTE_UNCACHED;
else
pte_flags |= GEN6_PTE_UNCACHED;
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 02/10] drm/i915: Add dev to ppgtt
2012-10-23 1:34 ` [PATCH 02/10] drm/i915: Add dev to ppgtt Ben Widawsky
@ 2012-10-25 20:48 ` Jesse Barnes
0 siblings, 0 replies; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 20:48 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:07 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> Some subsequent commits will need to know what generation we're running
> on to do different pte encoding for the ppgtt. Since it's not much
> hassle or overhead to store it in the ppgtt structure, do that.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index af0e97e..bf628c4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -344,6 +344,7 @@ struct intel_device_info {
> #define I915_PPGTT_PD_ENTRIES 512
> #define I915_PPGTT_PT_ENTRIES 1024
> struct i915_hw_ppgtt {
> + struct drm_device *dev;
> unsigned num_pd_entries;
> struct page **pt_pages;
> uint32_t pd_offset;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 5751ad2..2b75028 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -78,6 +78,7 @@ int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
> if (!ppgtt)
> return ret;
>
> + ppgtt->dev = dev;
> ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES;
> ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
> GFP_KERNEL);
> @@ -219,7 +220,7 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
> switch (cache_level) {
> case I915_CACHE_LLC_MLC:
> /* Haswell doesn't set L3 this way */
> - if (IS_HASWELL(obj->base.dev))
> + if (IS_HASWELL(ppgtt->dev))
> pte_flags |= GEN6_PTE_CACHE_LLC;
> else
> pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> @@ -228,7 +229,7 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
> pte_flags |= GEN6_PTE_CACHE_LLC;
> break;
> case I915_CACHE_NONE:
> - if (IS_HASWELL(obj->base.dev))
> + if (IS_HASWELL(ppgtt->dev))
> pte_flags |= HSW_PTE_UNCACHED;
> else
> pte_flags |= GEN6_PTE_UNCACHED;
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
It would be nice if you did some penance for bloating our structures a
little though, maybe by removing at least 3 fields from our dev_priv
struct? :)
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 03/10] drm/i915: introduce gtt_pte_t
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
2012-10-23 1:34 ` [PATCH 01/10] drm/i915: No LLC_MLC for HSW Ben Widawsky
2012-10-23 1:34 ` [PATCH 02/10] drm/i915: Add dev to ppgtt Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-25 20:49 ` Jesse Barnes
2012-10-23 1:34 ` [PATCH 04/10] drm/i915: Extract PPGTT pte encoding Ben Widawsky
` (7 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
This will make the calculations of size easier to read instead of just
assuming uint32_t everywhere.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2b75028..a769b3c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -29,13 +29,15 @@
#include "i915_trace.h"
#include "intel_drv.h"
+typedef uint32_t gtt_pte_t;
+
/* PPGTT support for Sandybdrige/Gen6 and later */
static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
unsigned first_entry,
unsigned num_entries)
{
- uint32_t *pt_vaddr;
- uint32_t scratch_pte;
+ gtt_pte_t *pt_vaddr;
+ gtt_pte_t scratch_pte;
unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
unsigned last_pte, i;
@@ -120,7 +122,7 @@ int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
i915_ppgtt_clear_range(ppgtt, 0,
ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
- ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(uint32_t);
+ ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(gtt_pte_t);
dev_priv->mm.aliasing_ppgtt = ppgtt;
@@ -170,9 +172,9 @@ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev)
static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
const struct sg_table *pages,
unsigned first_entry,
- uint32_t pte_flags)
+ gtt_pte_t pte_flags)
{
- uint32_t *pt_vaddr, pte;
+ gtt_pte_t *pt_vaddr, pte;
unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
unsigned i, j, m, segment_len;
@@ -215,7 +217,7 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
struct drm_i915_gem_object *obj,
enum i915_cache_level cache_level)
{
- uint32_t pte_flags = GEN6_PTE_VALID;
+ gtt_pte_t pte_flags = GEN6_PTE_VALID;
switch (cache_level) {
case I915_CACHE_LLC_MLC:
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 03/10] drm/i915: introduce gtt_pte_t
2012-10-23 1:34 ` [PATCH 03/10] drm/i915: introduce gtt_pte_t Ben Widawsky
@ 2012-10-25 20:49 ` Jesse Barnes
0 siblings, 0 replies; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 20:49 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:08 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> This will make the calculations of size easier to read instead of just
> assuming uint32_t everywhere.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 2b75028..a769b3c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -29,13 +29,15 @@
> #include "i915_trace.h"
> #include "intel_drv.h"
>
> +typedef uint32_t gtt_pte_t;
> +
> /* PPGTT support for Sandybdrige/Gen6 and later */
> static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> unsigned first_entry,
> unsigned num_entries)
> {
> - uint32_t *pt_vaddr;
> - uint32_t scratch_pte;
> + gtt_pte_t *pt_vaddr;
> + gtt_pte_t scratch_pte;
> unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
> unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
> unsigned last_pte, i;
> @@ -120,7 +122,7 @@ int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
> i915_ppgtt_clear_range(ppgtt, 0,
> ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
>
> - ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(uint32_t);
> + ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(gtt_pte_t);
>
> dev_priv->mm.aliasing_ppgtt = ppgtt;
>
> @@ -170,9 +172,9 @@ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev)
> static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
> const struct sg_table *pages,
> unsigned first_entry,
> - uint32_t pte_flags)
> + gtt_pte_t pte_flags)
> {
> - uint32_t *pt_vaddr, pte;
> + gtt_pte_t *pt_vaddr, pte;
> unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
> unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
> unsigned i, j, m, segment_len;
> @@ -215,7 +217,7 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
> struct drm_i915_gem_object *obj,
> enum i915_cache_level cache_level)
> {
> - uint32_t pte_flags = GEN6_PTE_VALID;
> + gtt_pte_t pte_flags = GEN6_PTE_VALID;
>
> switch (cache_level) {
> case I915_CACHE_LLC_MLC:
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 04/10] drm/i915: Extract PPGTT pte encoding
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
` (2 preceding siblings ...)
2012-10-23 1:34 ` [PATCH 03/10] drm/i915: introduce gtt_pte_t Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-25 20:50 ` Jesse Barnes
2012-10-23 1:34 ` [PATCH 05/10] drm/i915: move more pte encoding to pte encode Ben Widawsky
` (6 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
HSW will change the PTE encoding, and laying this out now will be
helpful when we're ready to implement that. More importantly, GGTT and
PPGTT PTE encoding is quite similar, so moving this out into a helper
function will enable us to lance the AGP layer.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a769b3c..da9c1fa 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -31,6 +31,17 @@
typedef uint32_t gtt_pte_t;
+static inline gtt_pte_t pte_encode(struct drm_device *dev,
+ dma_addr_t addr,
+ gtt_pte_t cache_bits)
+{
+ gtt_pte_t pte = GEN6_PTE_VALID;
+ pte |= GEN6_PTE_ADDR_ENCODE(addr);
+ pte |= cache_bits;
+
+ return pte;
+}
+
/* PPGTT support for Sandybdrige/Gen6 and later */
static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
unsigned first_entry,
@@ -42,8 +53,8 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
unsigned last_pte, i;
- scratch_pte = GEN6_PTE_ADDR_ENCODE(ppgtt->scratch_page_dma_addr);
- scratch_pte |= GEN6_PTE_VALID | GEN6_PTE_CACHE_LLC;
+ scratch_pte = pte_encode(ppgtt->dev, ppgtt->scratch_page_dma_addr,
+ GEN6_PTE_CACHE_LLC);
while (num_entries) {
last_pte = first_pte + num_entries;
@@ -174,7 +185,7 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
unsigned first_entry,
gtt_pte_t pte_flags)
{
- gtt_pte_t *pt_vaddr, pte;
+ gtt_pte_t *pt_vaddr;
unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
unsigned i, j, m, segment_len;
@@ -192,8 +203,8 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
for (j = first_pte; j < I915_PPGTT_PT_ENTRIES; j++) {
page_addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
- pte = GEN6_PTE_ADDR_ENCODE(page_addr);
- pt_vaddr[j] = pte | pte_flags;
+ pt_vaddr[j] = pte_encode(ppgtt->dev, page_addr,
+ pte_flags);
/* grab the next page */
if (++m == segment_len) {
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 04/10] drm/i915: Extract PPGTT pte encoding
2012-10-23 1:34 ` [PATCH 04/10] drm/i915: Extract PPGTT pte encoding Ben Widawsky
@ 2012-10-25 20:50 ` Jesse Barnes
0 siblings, 0 replies; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 20:50 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:09 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> HSW will change the PTE encoding, and laying this out now will be
> helpful when we're ready to implement that. More importantly, GGTT and
> PPGTT PTE encoding is quite similar, so moving this out into a helper
> function will enable us to lance the AGP layer.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a769b3c..da9c1fa 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -31,6 +31,17 @@
>
> typedef uint32_t gtt_pte_t;
>
> +static inline gtt_pte_t pte_encode(struct drm_device *dev,
> + dma_addr_t addr,
> + gtt_pte_t cache_bits)
> +{
> + gtt_pte_t pte = GEN6_PTE_VALID;
> + pte |= GEN6_PTE_ADDR_ENCODE(addr);
> + pte |= cache_bits;
> +
> + return pte;
> +}
> +
> /* PPGTT support for Sandybdrige/Gen6 and later */
> static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> unsigned first_entry,
> @@ -42,8 +53,8 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
> unsigned last_pte, i;
>
> - scratch_pte = GEN6_PTE_ADDR_ENCODE(ppgtt->scratch_page_dma_addr);
> - scratch_pte |= GEN6_PTE_VALID | GEN6_PTE_CACHE_LLC;
> + scratch_pte = pte_encode(ppgtt->dev, ppgtt->scratch_page_dma_addr,
> + GEN6_PTE_CACHE_LLC);
>
> while (num_entries) {
> last_pte = first_pte + num_entries;
> @@ -174,7 +185,7 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
> unsigned first_entry,
> gtt_pte_t pte_flags)
> {
> - gtt_pte_t *pt_vaddr, pte;
> + gtt_pte_t *pt_vaddr;
> unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
> unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
> unsigned i, j, m, segment_len;
> @@ -192,8 +203,8 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
>
> for (j = first_pte; j < I915_PPGTT_PT_ENTRIES; j++) {
> page_addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
> - pte = GEN6_PTE_ADDR_ENCODE(page_addr);
> - pt_vaddr[j] = pte | pte_flags;
> + pt_vaddr[j] = pte_encode(ppgtt->dev, page_addr,
> + pte_flags);
>
> /* grab the next page */
> if (++m == segment_len) {
Does this mean we can remove the GEN6_PTE_ADDR_ENCODE macro too?
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 05/10] drm/i915: move more pte encoding to pte encode
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
` (3 preceding siblings ...)
2012-10-23 1:34 ` [PATCH 04/10] drm/i915: Extract PPGTT pte encoding Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-25 20:51 ` Jesse Barnes
2012-10-23 1:34 ` [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+ Ben Widawsky
` (5 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
In order to handle differences in pte encoding between architectures it
is desirable to have one helper function, pte_encode, do it all for us.
As such, this commit moves the code around so we're in good shape to do
that.
Luckily the ppgtt pte and the ggtt pte look very similar.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 56 ++++++++++++++++++-------------------
1 file changed, 27 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index da9c1fa..89c273f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -33,11 +33,32 @@ typedef uint32_t gtt_pte_t;
static inline gtt_pte_t pte_encode(struct drm_device *dev,
dma_addr_t addr,
- gtt_pte_t cache_bits)
+ enum i915_cache_level level)
{
gtt_pte_t pte = GEN6_PTE_VALID;
pte |= GEN6_PTE_ADDR_ENCODE(addr);
- pte |= cache_bits;
+
+ switch (level) {
+ case I915_CACHE_LLC_MLC:
+ /* Haswell doesn't set L3 this way */
+ if (IS_HASWELL(dev))
+ pte |= GEN6_PTE_CACHE_LLC;
+ else
+ pte |= GEN6_PTE_CACHE_LLC_MLC;
+ break;
+ case I915_CACHE_LLC:
+ pte |= GEN6_PTE_CACHE_LLC;
+ break;
+ case I915_CACHE_NONE:
+ if (IS_HASWELL(dev))
+ pte |= HSW_PTE_UNCACHED;
+ else
+ pte |= GEN6_PTE_UNCACHED;
+ break;
+ default:
+ BUG();
+ }
+
return pte;
}
@@ -54,7 +75,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
unsigned last_pte, i;
scratch_pte = pte_encode(ppgtt->dev, ppgtt->scratch_page_dma_addr,
- GEN6_PTE_CACHE_LLC);
+ I915_CACHE_LLC);
while (num_entries) {
last_pte = first_pte + num_entries;
@@ -183,7 +204,7 @@ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev)
static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
const struct sg_table *pages,
unsigned first_entry,
- gtt_pte_t pte_flags)
+ enum i915_cache_level cache_level)
{
gtt_pte_t *pt_vaddr;
unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
@@ -204,7 +225,7 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
for (j = first_pte; j < I915_PPGTT_PT_ENTRIES; j++) {
page_addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
pt_vaddr[j] = pte_encode(ppgtt->dev, page_addr,
- pte_flags);
+ cache_level);
/* grab the next page */
if (++m == segment_len) {
@@ -228,33 +249,10 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
struct drm_i915_gem_object *obj,
enum i915_cache_level cache_level)
{
- gtt_pte_t pte_flags = GEN6_PTE_VALID;
-
- switch (cache_level) {
- case I915_CACHE_LLC_MLC:
- /* Haswell doesn't set L3 this way */
- if (IS_HASWELL(ppgtt->dev))
- pte_flags |= GEN6_PTE_CACHE_LLC;
- else
- pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
- break;
- case I915_CACHE_LLC:
- pte_flags |= GEN6_PTE_CACHE_LLC;
- break;
- case I915_CACHE_NONE:
- if (IS_HASWELL(ppgtt->dev))
- pte_flags |= HSW_PTE_UNCACHED;
- else
- pte_flags |= GEN6_PTE_UNCACHED;
- break;
- default:
- BUG();
- }
-
i915_ppgtt_insert_sg_entries(ppgtt,
obj->pages,
obj->gtt_space->start >> PAGE_SHIFT,
- pte_flags);
+ cache_level);
}
void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 05/10] drm/i915: move more pte encoding to pte encode
2012-10-23 1:34 ` [PATCH 05/10] drm/i915: move more pte encoding to pte encode Ben Widawsky
@ 2012-10-25 20:51 ` Jesse Barnes
2012-10-31 10:52 ` Daniel Vetter
0 siblings, 1 reply; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 20:51 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:10 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> In order to handle differences in pte encoding between architectures it
> is desirable to have one helper function, pte_encode, do it all for us.
> As such, this commit moves the code around so we're in good shape to do
> that.
>
> Luckily the ppgtt pte and the ggtt pte look very similar.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 56 ++++++++++++++++++-------------------
> 1 file changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index da9c1fa..89c273f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -33,11 +33,32 @@ typedef uint32_t gtt_pte_t;
>
> static inline gtt_pte_t pte_encode(struct drm_device *dev,
> dma_addr_t addr,
> - gtt_pte_t cache_bits)
> + enum i915_cache_level level)
> {
> gtt_pte_t pte = GEN6_PTE_VALID;
> pte |= GEN6_PTE_ADDR_ENCODE(addr);
> - pte |= cache_bits;
> +
> + switch (level) {
> + case I915_CACHE_LLC_MLC:
> + /* Haswell doesn't set L3 this way */
> + if (IS_HASWELL(dev))
> + pte |= GEN6_PTE_CACHE_LLC;
> + else
> + pte |= GEN6_PTE_CACHE_LLC_MLC;
> + break;
> + case I915_CACHE_LLC:
> + pte |= GEN6_PTE_CACHE_LLC;
> + break;
> + case I915_CACHE_NONE:
> + if (IS_HASWELL(dev))
> + pte |= HSW_PTE_UNCACHED;
> + else
> + pte |= GEN6_PTE_UNCACHED;
> + break;
> + default:
> + BUG();
> + }
> +
>
> return pte;
> }
> @@ -54,7 +75,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> unsigned last_pte, i;
>
> scratch_pte = pte_encode(ppgtt->dev, ppgtt->scratch_page_dma_addr,
> - GEN6_PTE_CACHE_LLC);
> + I915_CACHE_LLC);
>
> while (num_entries) {
> last_pte = first_pte + num_entries;
> @@ -183,7 +204,7 @@ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev)
> static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
> const struct sg_table *pages,
> unsigned first_entry,
> - gtt_pte_t pte_flags)
> + enum i915_cache_level cache_level)
> {
> gtt_pte_t *pt_vaddr;
> unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
> @@ -204,7 +225,7 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
> for (j = first_pte; j < I915_PPGTT_PT_ENTRIES; j++) {
> page_addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
> pt_vaddr[j] = pte_encode(ppgtt->dev, page_addr,
> - pte_flags);
> + cache_level);
>
> /* grab the next page */
> if (++m == segment_len) {
> @@ -228,33 +249,10 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
> struct drm_i915_gem_object *obj,
> enum i915_cache_level cache_level)
> {
> - gtt_pte_t pte_flags = GEN6_PTE_VALID;
> -
> - switch (cache_level) {
> - case I915_CACHE_LLC_MLC:
> - /* Haswell doesn't set L3 this way */
> - if (IS_HASWELL(ppgtt->dev))
> - pte_flags |= GEN6_PTE_CACHE_LLC;
> - else
> - pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> - break;
> - case I915_CACHE_LLC:
> - pte_flags |= GEN6_PTE_CACHE_LLC;
> - break;
> - case I915_CACHE_NONE:
> - if (IS_HASWELL(ppgtt->dev))
> - pte_flags |= HSW_PTE_UNCACHED;
> - else
> - pte_flags |= GEN6_PTE_UNCACHED;
> - break;
> - default:
> - BUG();
> - }
> -
> i915_ppgtt_insert_sg_entries(ppgtt,
> obj->pages,
> obj->gtt_space->start >> PAGE_SHIFT,
> - pte_flags);
> + cache_level);
> }
>
> void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 05/10] drm/i915: move more pte encoding to pte encode
2012-10-25 20:51 ` Jesse Barnes
@ 2012-10-31 10:52 ` Daniel Vetter
0 siblings, 0 replies; 50+ messages in thread
From: Daniel Vetter @ 2012-10-31 10:52 UTC (permalink / raw)
To: Jesse Barnes; +Cc: Ben Widawsky, intel-gfx
On Thu, Oct 25, 2012 at 01:51:27PM -0700, Jesse Barnes wrote:
> On Mon, 22 Oct 2012 18:34:10 -0700
> Ben Widawsky <ben@bwidawsk.net> wrote:
>
> > In order to handle differences in pte encoding between architectures it
> > is desirable to have one helper function, pte_encode, do it all for us.
> > As such, this commit moves the code around so we're in good shape to do
> > that.
> >
> > Luckily the ppgtt pte and the ggtt pte look very similar.
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Queued for -next up to this patch, thanks.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
` (4 preceding siblings ...)
2012-10-23 1:34 ` [PATCH 05/10] drm/i915: move more pte encoding to pte encode Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-23 9:59 ` Chris Wilson
` (2 more replies)
2012-10-23 1:34 ` [PATCH 07/10] drm/i915: Calculate correct stolen size for GEN7+ Ben Widawsky
` (4 subsequent siblings)
10 siblings, 3 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
As a quick hack we make the old intel_gtt structure mutable so we can
fool a bunch of the existing code which depends on elements in that data
structure. We can/should try to remove this in a subsequent patch.
This should preserve the old gtt init behavior which upon writing these
patches seems incorrect. The next patch will fix these things.
The one exception is VLV which doesn't have the preserved flush control
write behavior. Since we want to do that for all GEN6+ stuff, we'll
handle that in a later patch. Mainstream VLV support doesn't actually
exist yet anyway.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/char/agp/intel-gtt.c | 2 +-
drivers/gpu/drm/i915/i915_dma.c | 16 +--
drivers/gpu/drm/i915/i915_drv.h | 10 +-
drivers/gpu/drm/i915/i915_gem.c | 12 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 208 +++++++++++++++++++++++++++--
drivers/gpu/drm/i915/i915_reg.h | 6 +
include/drm/intel-gtt.h | 3 +-
8 files changed, 228 insertions(+), 31 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 38390f7..4dfbb80 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1686,7 +1686,7 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
}
EXPORT_SYMBOL(intel_gmch_probe);
-const struct intel_gtt *intel_gtt_get(void)
+struct intel_gtt *intel_gtt_get(void)
{
return &intel_private.base;
}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ffbc915..eda983b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1492,19 +1492,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
goto free_priv;
}
- ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
- if (!ret) {
- DRM_ERROR("failed to set up gmch\n");
- ret = -EIO;
+ ret = i915_gem_gtt_init(dev);
+ if (ret)
goto put_bridge;
- }
-
- dev_priv->mm.gtt = intel_gtt_get();
- if (!dev_priv->mm.gtt) {
- DRM_ERROR("Failed to initialize GTT\n");
- ret = -ENODEV;
- goto put_gmch;
- }
i915_kick_out_firmware_fb(dev_priv);
@@ -1678,7 +1668,7 @@ out_mtrrfree:
out_rmmap:
pci_iounmap(dev->pdev, dev_priv->regs);
put_gmch:
- intel_gmch_remove();
+ i915_gem_gtt_fini(dev);
put_bridge:
pci_dev_put(dev_priv->bridge_dev);
free_priv:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bf628c4..534d282 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -687,7 +687,7 @@ typedef struct drm_i915_private {
struct {
/** Bridge to intel-gtt-ko */
- const struct intel_gtt *gtt;
+ struct intel_gtt *gtt;
/** Memory allocator for GTT stolen memory */
struct drm_mm stolen;
/** Memory allocator for GTT */
@@ -1507,6 +1507,14 @@ void i915_gem_init_global_gtt(struct drm_device *dev,
unsigned long start,
unsigned long mappable_end,
unsigned long end);
+int i915_gem_gtt_init(struct drm_device *dev);
+void i915_gem_gtt_fini(struct drm_device *dev);
+extern inline void i915_gem_chipset_flush(struct drm_device *dev)
+{
+ if (INTEL_INFO(dev)->gen < 6)
+ intel_gtt_chipset_flush();
+}
+
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct drm_device *dev, int min_size,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e04820c..ae3d4c1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -846,12 +846,12 @@ out:
* domain anymore. */
if (obj->base.write_domain != I915_GEM_DOMAIN_CPU) {
i915_gem_clflush_object(obj);
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
}
}
if (needs_clflush_after)
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
return ret;
}
@@ -3059,7 +3059,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj)
return;
i915_gem_clflush_object(obj);
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(obj->base.dev);
old_write_domain = obj->base.write_domain;
obj->base.write_domain = 0;
@@ -3960,7 +3960,7 @@ i915_gem_init_hw(struct drm_device *dev)
drm_i915_private_t *dev_priv = dev->dev_private;
int ret;
- if (!intel_enable_gtt())
+ if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
return -EIO;
if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1))
@@ -4295,7 +4295,7 @@ void i915_gem_detach_phys_object(struct drm_device *dev,
page_cache_release(page);
}
}
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
obj->phys_obj->cur_obj = NULL;
obj->phys_obj = NULL;
@@ -4382,7 +4382,7 @@ i915_gem_phys_pwrite(struct drm_device *dev,
return -EFAULT;
}
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 6a2f3e5..1af00b5 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -673,7 +673,7 @@ i915_gem_execbuffer_move_to_gpu(struct intel_ring_buffer *ring,
}
if (flush_domains & I915_GEM_DOMAIN_CPU)
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(ring->dev);
if (flush_domains & I915_GEM_DOMAIN_GTT)
wmb();
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 89c273f..7d3ec42 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -305,13 +305,38 @@ static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible)
dev_priv->mm.interruptible = interruptible;
}
+
+static void i915_ggtt_clear_range(struct drm_device *dev,
+ unsigned first_entry,
+ unsigned num_entries)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ gtt_pte_t scratch_pte;
+ volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
+ const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
+
+ if (INTEL_INFO(dev)->gen < 6) {
+ intel_gtt_clear_range(first_entry, num_entries);
+ return;
+ }
+
+ if (WARN(num_entries > max_entries,
+ "First entry = %d; Num entries = %d (max=%d)\n",
+ first_entry, num_entries, max_entries))
+ num_entries = max_entries;
+
+ scratch_pte = pte_encode(dev, dev_priv->mm.gtt->scratch_page_dma, I915_CACHE_LLC);
+ memset_io(gtt_base, scratch_pte, num_entries * sizeof(scratch_pte));
+ readl(gtt_base);
+}
+
void i915_gem_restore_gtt_mappings(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj;
/* First fill our portion of the GTT with scratch pages */
- intel_gtt_clear_range(dev_priv->mm.gtt_start / PAGE_SIZE,
+ i915_ggtt_clear_range(dev, dev_priv->mm.gtt_start / PAGE_SIZE,
(dev_priv->mm.gtt_end - dev_priv->mm.gtt_start) / PAGE_SIZE);
list_for_each_entry(obj, &dev_priv->mm.bound_list, gtt_list) {
@@ -319,7 +344,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
i915_gem_gtt_bind_object(obj, obj->cache_level);
}
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
}
int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
@@ -335,21 +360,64 @@ int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
return 0;
}
+/*
+ * Binds an object into the global gtt with the specified cache level. The object
+ * will be accessible to the GPU via commands whose operands reference offsets
+ * within the global GTT as well as accessible by the GPU through the GMADR
+ * mapped BAR (dev_priv->mm.gtt->gtt).
+ */
+static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
+ enum i915_cache_level level)
+{
+ struct drm_device *dev = obj->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct sg_table *st = obj->pages;
+ struct scatterlist *sg = st->sgl;
+ const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
+ const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
+ gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gtt->gtt + first_entry;
+ int unused, i = 0;
+ unsigned int len, m = 0;
+
+ for_each_sg(st->sgl, sg, st->nents, unused) {
+ len = sg_dma_len(sg) >> PAGE_SHIFT;
+ for (m = 0; m < len; m++) {
+ dma_addr_t addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
+ gtt_entries[i] = pte_encode(dev, addr, level);
+ i++;
+ if (WARN_ON(i > max_entries))
+ goto out;
+ }
+ }
+
+out:
+ /* XXX: This serves as a posting read preserving the way the old code
+ * works. It's not clear if this is strictly necessary or just voodoo
+ * based on what I've tried to gather from the docs.
+ */
+ readl(>t_entries[i-1]);
+}
+
void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
enum i915_cache_level cache_level)
{
struct drm_device *dev = obj->base.dev;
- unsigned int agp_type = cache_level_to_agp_type(dev, cache_level);
+ if (INTEL_INFO(dev)->gen < 6) {
+ unsigned int agp_type = cache_level_to_agp_type(dev, cache_level);
+ intel_gtt_insert_sg_entries(obj->pages,
+ obj->gtt_space->start >> PAGE_SHIFT,
+ agp_type);
+ } else {
+ gen6_ggtt_bind_object(obj, cache_level);
+ }
- intel_gtt_insert_sg_entries(obj->pages,
- obj->gtt_space->start >> PAGE_SHIFT,
- agp_type);
obj->has_global_gtt_mapping = 1;
}
void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
{
- intel_gtt_clear_range(obj->gtt_space->start >> PAGE_SHIFT,
+ i915_ggtt_clear_range(obj->base.dev,
+ obj->gtt_space->start >> PAGE_SHIFT,
obj->base.size >> PAGE_SHIFT);
obj->has_global_gtt_mapping = 0;
@@ -407,5 +475,129 @@ void i915_gem_init_global_gtt(struct drm_device *dev,
dev_priv->mm.mappable_gtt_total = min(end, mappable_end) - start;
/* ... but ensure that we clear the entire range. */
- intel_gtt_clear_range(start / PAGE_SIZE, (end-start) / PAGE_SIZE);
+ i915_ggtt_clear_range(dev, start / PAGE_SIZE, (end-start) / PAGE_SIZE);
+}
+
+static int setup_scratch_page(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct page *page;
+ dma_addr_t dma_addr;
+
+ page = alloc_page(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO);
+ if (page == NULL)
+ return -ENOMEM;
+ get_page(page);
+ set_pages_uc(page, 1);
+
+#ifdef CONFIG_INTEL_IOMMU
+ dma_addr = pci_map_page(dev->pdev, page, 0, PAGE_SIZE,
+ PCI_DMA_BIDIRECTIONAL);
+ if (pci_dma_mapping_error(dev->pdev, dma_addr))
+ return -EINVAL;
+#else
+ dma_addr = page_to_phys(page);
+#endif
+ dev_priv->mm.gtt->scratch_page = page;
+ dev_priv->mm.gtt->scratch_page_dma = dma_addr;
+
+ return 0;
+}
+
+static void teardown_scratch_page(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ set_pages_wb(dev_priv->mm.gtt->scratch_page, 1);
+ pci_unmap_page(dev->pdev, dev_priv->mm.gtt->scratch_page_dma,
+ PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
+ put_page(dev_priv->mm.gtt->scratch_page);
+ __free_page(dev_priv->mm.gtt->scratch_page);
+}
+
+static inline unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
+{
+ snb_gmch_ctl >>= SNB_GMCH_GGMS_SHIFT;
+ snb_gmch_ctl &= SNB_GMCH_GGMS_MASK;
+ return snb_gmch_ctl << 20;
+}
+
+static inline unsigned int gen6_get_stolen_size(u16 snb_gmch_ctl)
+{
+ snb_gmch_ctl >>= SNB_GMCH_GMS_SHIFT;
+ snb_gmch_ctl &= SNB_GMCH_GMS_MASK;
+ return snb_gmch_ctl << 25; /* 32 MB units */
+}
+
+int i915_gem_gtt_init(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ phys_addr_t gtt_bus_addr;
+ u16 snb_gmch_ctl;
+ u32 tmp;
+ int ret;
+
+ /* On modern platforms we need not worry ourself with the legacy
+ * hostbridge query stuff. Skip it entirely
+ */
+ if (INTEL_INFO(dev)->gen < 6) {
+ ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
+ if (!ret) {
+ DRM_ERROR("failed to set up gmch\n");
+ return -EIO;
+ }
+
+ dev_priv->mm.gtt = intel_gtt_get();
+ if (!dev_priv->mm.gtt) {
+ DRM_ERROR("Failed to initialize GTT\n");
+ intel_gmch_remove();
+ return -ENODEV;
+ }
+ return 0;
+ }
+
+
+ dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
+ if (!dev_priv->mm.gtt)
+ return -ENOMEM;
+
+ if (!pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(40)))
+ pci_set_consistent_dma_mask(dev->pdev, DMA_BIT_MASK(40));
+
+ pci_read_config_dword(dev->pdev, PCI_BASE_ADDRESS_0, &tmp);
+ /* For GEN6+ the PTEs for the ggtt live at 2MB + BAR0 */
+ gtt_bus_addr = (tmp & PCI_BASE_ADDRESS_MEM_MASK) + (2<<20);
+
+ dev_priv->mm.gtt->gtt_mappable_entries = pci_resource_len(dev->pdev, 2) >> PAGE_SHIFT;
+ pci_read_config_dword(dev->pdev, PCI_BASE_ADDRESS_2, &tmp);
+ dev_priv->mm.gtt->gma_bus_addr = tmp & PCI_BASE_ADDRESS_MEM_MASK;
+
+ /* i9xx_setup */
+ pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
+ dev_priv->mm.gtt->gtt_total_entries =
+ gen6_get_total_gtt_size(snb_gmch_ctl) / sizeof(gtt_pte_t);
+ dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
+
+ ret = setup_scratch_page(dev);
+ if (ret)
+ return ret;
+
+ dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
+ dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
+
+ /* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
+ DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
+ DRM_DEBUG_DRIVER("GTT total entries = %dK\n", dev_priv->mm.gtt->gtt_total_entries >> 10);
+ DRM_DEBUG_DRIVER("GTT stolen size = %dM\n", dev_priv->mm.gtt->stolen_size >> 20);
+
+ return 0;
+}
+
+void i915_gem_gtt_fini(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ iounmap(dev_priv->mm.gtt->gtt);
+ teardown_scratch_page(dev);
+ if (INTEL_INFO(dev)->gen < 6)
+ intel_gmch_remove();
+ kfree(dev_priv->mm.gtt);
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f4d70b0..10a6e9b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -40,6 +40,12 @@
*/
#define INTEL_GMCH_CTRL 0x52
#define INTEL_GMCH_VGA_DISABLE (1 << 1)
+#define SNB_GMCH_CTRL 0x50
+#define SNB_GMCH_GGMS_SHIFT 8 /* GTT Graphics Memory Size */
+#define SNB_GMCH_GGMS_MASK 0x3
+#define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */
+#define SNB_GMCH_GMS_MASK 0x1f
+
/* PCI config space */
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 2e37e9f..94e8f2c 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -3,7 +3,7 @@
#ifndef _DRM_INTEL_GTT_H
#define _DRM_INTEL_GTT_H
-const struct intel_gtt {
+struct intel_gtt {
/* Size of memory reserved for graphics by the BIOS */
unsigned int stolen_size;
/* Total number of gtt entries. */
@@ -17,6 +17,7 @@ const struct intel_gtt {
unsigned int do_idle_maps : 1;
/* Share the scratch page dma with ppgtts. */
dma_addr_t scratch_page_dma;
+ struct page *scratch_page;
/* for ppgtt PDE access */
u32 __iomem *gtt;
/* needed for ioremap in drm/i915 */
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 1:34 ` [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+ Ben Widawsky
@ 2012-10-23 9:59 ` Chris Wilson
2012-10-23 14:57 ` Ben Widawsky
[not found] ` <878vaxwk61.fsf@gaia.fi.intel.com>
2012-10-26 20:54 ` [PATCH 06/10 v2] " Ben Widawsky
2 siblings, 1 reply; 50+ messages in thread
From: Chris Wilson @ 2012-10-23 9:59 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Mon, 22 Oct 2012 18:34:11 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> +/*
> + * Binds an object into the global gtt with the specified cache level. The object
> + * will be accessible to the GPU via commands whose operands reference offsets
> + * within the global GTT as well as accessible by the GPU through the GMADR
> + * mapped BAR (dev_priv->mm.gtt->gtt).
> + */
> +static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
> + enum i915_cache_level level)
> +{
> + struct drm_device *dev = obj->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct sg_table *st = obj->pages;
> + struct scatterlist *sg = st->sgl;
> + const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
> + const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
> + gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gtt->gtt + first_entry;
> + int unused, i = 0;
> + unsigned int len, m = 0;
> +
> + for_each_sg(st->sgl, sg, st->nents, unused) {
> + len = sg_dma_len(sg) >> PAGE_SHIFT;
> + for (m = 0; m < len; m++) {
> + dma_addr_t addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
> + gtt_entries[i] = pte_encode(dev, addr, level);
> + i++;
> + if (WARN_ON(i > max_entries))
> + goto out;
> + }
> + }
> +
> +out:
> + /* XXX: This serves as a posting read preserving the way the old code
> + * works. It's not clear if this is strictly necessary or just voodoo
> + * based on what I've tried to gather from the docs.
> + */
> + readl(>t_entries[i-1]);
It will be required until we replace the voodoo with more explicit mb().
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 9:59 ` Chris Wilson
@ 2012-10-23 14:57 ` Ben Widawsky
2012-10-23 14:58 ` Daniel Vetter
2012-10-25 20:54 ` Jesse Barnes
0 siblings, 2 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 14:57 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On 2012-10-23 02:59, Chris Wilson wrote:
> On Mon, 22 Oct 2012 18:34:11 -0700, Ben Widawsky <ben@bwidawsk.net>
> wrote:
>> +/*
>> + * Binds an object into the global gtt with the specified cache
>> level. The object
>> + * will be accessible to the GPU via commands whose operands
>> reference offsets
>> + * within the global GTT as well as accessible by the GPU through
>> the GMADR
>> + * mapped BAR (dev_priv->mm.gtt->gtt).
>> + */
>> +static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
>> + enum i915_cache_level level)
>> +{
>> + struct drm_device *dev = obj->base.dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + struct sg_table *st = obj->pages;
>> + struct scatterlist *sg = st->sgl;
>> + const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
>> + const int max_entries = dev_priv->mm.gtt->gtt_total_entries -
>> first_entry;
>> + gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gtt->gtt +
>> first_entry;
>> + int unused, i = 0;
>> + unsigned int len, m = 0;
>> +
>> + for_each_sg(st->sgl, sg, st->nents, unused) {
>> + len = sg_dma_len(sg) >> PAGE_SHIFT;
>> + for (m = 0; m < len; m++) {
>> + dma_addr_t addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
>> + gtt_entries[i] = pte_encode(dev, addr, level);
>> + i++;
>> + if (WARN_ON(i > max_entries))
>> + goto out;
>> + }
>> + }
>> +
>> +out:
>> + /* XXX: This serves as a posting read preserving the way the old
>> code
>> + * works. It's not clear if this is strictly necessary or just
>> voodoo
>> + * based on what I've tried to gather from the docs.
>> + */
>> + readl(>t_entries[i-1]);
>
> It will be required until we replace the voodoo with more explicit
> mb().
> -Chris
Actually, after we introduce the FLSH_CNTL patch from Jesse/me later in
the series, I think we just want a POSTING_READ on that register. It is
technically "required" by our desire to some day WC the registers, and
should synchronize everything else for us.
After a quick read of memory_barriers.txt (again), I think mmiowb is
actually what we might want in addition to the POSTING_READ I'd add.
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 14:57 ` Ben Widawsky
@ 2012-10-23 14:58 ` Daniel Vetter
2012-10-23 19:57 ` Ben Widawsky
2012-10-25 20:54 ` Jesse Barnes
1 sibling, 1 reply; 50+ messages in thread
From: Daniel Vetter @ 2012-10-23 14:58 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Tue, Oct 23, 2012 at 4:57 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
> Actually, after we introduce the FLSH_CNTL patch from Jesse/me later in the
> series, I think we just want a POSTING_READ on that register. It is
> technically "required" by our desire to some day WC the registers, and
> should synchronize everything else for us.
>
> After a quick read of memory_barriers.txt (again), I think mmiowb is
> actually what we might want in addition to the POSTING_READ I'd add.
Imo we have special rules for the igd, since clearly not all registers
in our 4mb mmio window are equal. So I'd prefer the keep the readback
of the last pte write (to ensure those are flushed out) and maybe also
add a readback of the gfx_flsh_cntl reg (like I've seen in some
internal vlv tree). Just to be paranoid.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 14:58 ` Daniel Vetter
@ 2012-10-23 19:57 ` Ben Widawsky
2012-10-23 20:03 ` Daniel Vetter
2012-10-25 20:55 ` Jesse Barnes
0 siblings, 2 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 19:57 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Tue, 23 Oct 2012 16:58:50 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Oct 23, 2012 at 4:57 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
> > Actually, after we introduce the FLSH_CNTL patch from Jesse/me later in the
> > series, I think we just want a POSTING_READ on that register. It is
> > technically "required" by our desire to some day WC the registers, and
> > should synchronize everything else for us.
> >
> > After a quick read of memory_barriers.txt (again), I think mmiowb is
> > actually what we might want in addition to the POSTING_READ I'd add.
>
> Imo we have special rules for the igd, since clearly not all registers
> in our 4mb mmio window are equal. So I'd prefer the keep the readback
> of the last pte write (to ensure those are flushed out) and maybe also
> add a readback of the gfx_flsh_cntl reg (like I've seen in some
> internal vlv tree). Just to be paranoid.
> -Daniel
What's your definition of flush? I think we just need one read to
satisfy the device I/O flush, and I think the write to the regsiter
should satisy the TLB flush. That leads me to the conclusion that just
the POSTING_READ of the FLSH_CNTL register is sufficient.
If you want me to do both, I have no problem with that either, and I'll
just update the comment to say that we believe it is unnecessary.
I don't really care either way.
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 19:57 ` Ben Widawsky
@ 2012-10-23 20:03 ` Daniel Vetter
2012-10-23 20:27 ` Ben Widawsky
2012-10-25 20:55 ` Jesse Barnes
1 sibling, 1 reply; 50+ messages in thread
From: Daniel Vetter @ 2012-10-23 20:03 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Tue, Oct 23, 2012 at 9:57 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
>> On Tue, Oct 23, 2012 at 4:57 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
>> > Actually, after we introduce the FLSH_CNTL patch from Jesse/me later in the
>> > series, I think we just want a POSTING_READ on that register. It is
>> > technically "required" by our desire to some day WC the registers, and
>> > should synchronize everything else for us.
>> >
>> > After a quick read of memory_barriers.txt (again), I think mmiowb is
>> > actually what we might want in addition to the POSTING_READ I'd add.
>>
>> Imo we have special rules for the igd, since clearly not all registers
>> in our 4mb mmio window are equal. So I'd prefer the keep the readback
>> of the last pte write (to ensure those are flushed out) and maybe also
>> add a readback of the gfx_flsh_cntl reg (like I've seen in some
>> internal vlv tree). Just to be paranoid.
>> -Daniel
>
> What's your definition of flush? I think we just need one read to
> satisfy the device I/O flush, and I think the write to the regsiter
> should satisy the TLB flush. That leads me to the conclusion that just
> the POSTING_READ of the FLSH_CNTL register is sufficient.
>
> If you want me to do both, I have no problem with that either, and I'll
> just update the comment to say that we believe it is unnecessary.
>
> I don't really care either way.
I think we need both, otherwise one of the pte writes might not have
arrived at the endpoint when the chip is purging the tlb already,
which might give is a tiny window where we load an invalid entry into
the tlb. Tiny window, but I'm paranoid ;-) And yes, I know that's more
strict than what docs say, but our igd also seems to be less coherent
than what docs claim.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 20:03 ` Daniel Vetter
@ 2012-10-23 20:27 ` Ben Widawsky
0 siblings, 0 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 20:27 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Tue, 23 Oct 2012 22:03:21 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Oct 23, 2012 at 9:57 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
> >> On Tue, Oct 23, 2012 at 4:57 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
> >> > Actually, after we introduce the FLSH_CNTL patch from Jesse/me later in the
> >> > series, I think we just want a POSTING_READ on that register. It is
> >> > technically "required" by our desire to some day WC the registers, and
> >> > should synchronize everything else for us.
> >> >
> >> > After a quick read of memory_barriers.txt (again), I think mmiowb is
> >> > actually what we might want in addition to the POSTING_READ I'd add.
> >>
> >> Imo we have special rules for the igd, since clearly not all registers
> >> in our 4mb mmio window are equal. So I'd prefer the keep the readback
> >> of the last pte write (to ensure those are flushed out) and maybe also
> >> add a readback of the gfx_flsh_cntl reg (like I've seen in some
> >> internal vlv tree). Just to be paranoid.
> >> -Daniel
> >
> > What's your definition of flush? I think we just need one read to
> > satisfy the device I/O flush, and I think the write to the regsiter
> > should satisy the TLB flush. That leads me to the conclusion that just
> > the POSTING_READ of the FLSH_CNTL register is sufficient.
> >
> > If you want me to do both, I have no problem with that either, and I'll
> > just update the comment to say that we believe it is unnecessary.
> >
> > I don't really care either way.
>
> I think we need both, otherwise one of the pte writes might not have
> arrived at the endpoint when the chip is purging the tlb already,
> which might give is a tiny window where we load an invalid entry into
> the tlb. Tiny window, but I'm paranoid ;-) And yes, I know that's more
> strict than what docs say, but our igd also seems to be less coherent
> than what docs claim.
> -Daniel
Hmm, that's a good point. If we write the last pte, and then flsh_cntl,
and then posting_read on flsh_cntl; I think it is also fine. (Unless
writes can be reordered)
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 19:57 ` Ben Widawsky
2012-10-23 20:03 ` Daniel Vetter
@ 2012-10-25 20:55 ` Jesse Barnes
1 sibling, 0 replies; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 20:55 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Tue, 23 Oct 2012 12:57:24 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> On Tue, 23 Oct 2012 16:58:50 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > On Tue, Oct 23, 2012 at 4:57 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
> > > Actually, after we introduce the FLSH_CNTL patch from Jesse/me later in the
> > > series, I think we just want a POSTING_READ on that register. It is
> > > technically "required" by our desire to some day WC the registers, and
> > > should synchronize everything else for us.
> > >
> > > After a quick read of memory_barriers.txt (again), I think mmiowb is
> > > actually what we might want in addition to the POSTING_READ I'd add.
> >
> > Imo we have special rules for the igd, since clearly not all registers
> > in our 4mb mmio window are equal. So I'd prefer the keep the readback
> > of the last pte write (to ensure those are flushed out) and maybe also
> > add a readback of the gfx_flsh_cntl reg (like I've seen in some
> > internal vlv tree). Just to be paranoid.
> > -Daniel
>
> What's your definition of flush? I think we just need one read to
> satisfy the device I/O flush, and I think the write to the regsiter
> should satisy the TLB flush. That leads me to the conclusion that just
> the POSTING_READ of the FLSH_CNTL register is sufficient.
>
> If you want me to do both, I have no problem with that either, and I'll
> just update the comment to say that we believe it is unnecessary.
>
> I don't really care either way.
Yeah we just need one or the other, not both. Technically the UC write
of the flush control reg should flush the WC buffer, then a posting
read of the flush control reg should make sure it gets to device 2
before the read returns.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 14:57 ` Ben Widawsky
2012-10-23 14:58 ` Daniel Vetter
@ 2012-10-25 20:54 ` Jesse Barnes
1 sibling, 0 replies; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 20:54 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Tue, 23 Oct 2012 07:57:12 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> On 2012-10-23 02:59, Chris Wilson wrote:
> > On Mon, 22 Oct 2012 18:34:11 -0700, Ben Widawsky <ben@bwidawsk.net>
> > wrote:
> >> +/*
> >> + * Binds an object into the global gtt with the specified cache
> >> level. The object
> >> + * will be accessible to the GPU via commands whose operands
> >> reference offsets
> >> + * within the global GTT as well as accessible by the GPU through
> >> the GMADR
> >> + * mapped BAR (dev_priv->mm.gtt->gtt).
> >> + */
> >> +static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
> >> + enum i915_cache_level level)
> >> +{
> >> + struct drm_device *dev = obj->base.dev;
> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> + struct sg_table *st = obj->pages;
> >> + struct scatterlist *sg = st->sgl;
> >> + const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
> >> + const int max_entries = dev_priv->mm.gtt->gtt_total_entries -
> >> first_entry;
> >> + gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gtt->gtt +
> >> first_entry;
> >> + int unused, i = 0;
> >> + unsigned int len, m = 0;
> >> +
> >> + for_each_sg(st->sgl, sg, st->nents, unused) {
> >> + len = sg_dma_len(sg) >> PAGE_SHIFT;
> >> + for (m = 0; m < len; m++) {
> >> + dma_addr_t addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
> >> + gtt_entries[i] = pte_encode(dev, addr, level);
> >> + i++;
> >> + if (WARN_ON(i > max_entries))
> >> + goto out;
> >> + }
> >> + }
> >> +
> >> +out:
> >> + /* XXX: This serves as a posting read preserving the way the old
> >> code
> >> + * works. It's not clear if this is strictly necessary or just
> >> voodoo
> >> + * based on what I've tried to gather from the docs.
> >> + */
> >> + readl(>t_entries[i-1]);
> >
> > It will be required until we replace the voodoo with more explicit
> > mb().
> > -Chris
>
> Actually, after we introduce the FLSH_CNTL patch from Jesse/me later in
> the series, I think we just want a POSTING_READ on that register. It is
> technically "required" by our desire to some day WC the registers, and
> should synchronize everything else for us.
>
> After a quick read of memory_barriers.txt (again), I think mmiowb is
> actually what we might want in addition to the POSTING_READ I'd add.
On a big NUMA system maybe (i.e. on nothing we run on yet), but on x86
mmiowb doesn't do anything other than act as a compiler optimization
barrier.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <878vaxwk61.fsf@gaia.fi.intel.com>]
* Re: [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+
[not found] ` <878vaxwk61.fsf@gaia.fi.intel.com>
@ 2012-10-24 17:56 ` Ben Widawsky
0 siblings, 0 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-24 17:56 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
On Tue, 23 Oct 2012 13:35:18 +0300
Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> On Mon, 22 Oct 2012 18:34:11 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > +static void teardown_scratch_page(struct drm_device *dev)
> > +{
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + set_pages_wb(dev_priv->mm.gtt->scratch_page, 1);
> > + pci_unmap_page(dev->pdev, dev_priv->mm.gtt->scratch_page_dma,
> > + PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
>
> #ifdef CONFIG_INTEL_IOMMU needed around pci_unmap_page?
It preserves the original behavior, which I believe is fine as is. The
iommu code should just do nothing. I am partial thuogh, since I am the
original reviewer on the patch that introduced that.
>
> > + put_page(dev_priv->mm.gtt->scratch_page);
> > + __free_page(dev_priv->mm.gtt->scratch_page);
> > +}
> > +
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 06/10 v2] drm/i915: Stop using AGP layer for GEN6+
2012-10-23 1:34 ` [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+ Ben Widawsky
2012-10-23 9:59 ` Chris Wilson
[not found] ` <878vaxwk61.fsf@gaia.fi.intel.com>
@ 2012-10-26 20:54 ` Ben Widawsky
2012-10-29 2:08 ` [PATCH 6/9 v3] " Ben Widawsky
2012-10-31 10:37 ` [PATCH 06/10 v2] " Chris Wilson
2 siblings, 2 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-26 20:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
As a quick hack we make the old intel_gtt structure mutable so we can
fool a bunch of the existing code which depends on elements in that data
structure. We can/should try to remove this in a subsequent patch.
This should preserve the old gtt init behavior which upon writing these
patches seems incorrect. The next patch will fix these things.
The one exception is VLV which doesn't have the preserved flush control
write behavior. Since we want to do that for all GEN6+ stuff, we'll
handle that in a later patch. Mainstream VLV support doesn't actually
exist yet anyway.
v2: Update the comment to remove the "voodoo"
Check that the last pte written matches what we readback
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/char/agp/intel-gtt.c | 2 +-
drivers/gpu/drm/i915/i915_dma.c | 16 +--
drivers/gpu/drm/i915/i915_drv.h | 10 +-
drivers/gpu/drm/i915/i915_gem.c | 12 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 214 +++++++++++++++++++++++++++--
drivers/gpu/drm/i915/i915_reg.h | 6 +
include/drm/intel-gtt.h | 3 +-
8 files changed, 234 insertions(+), 31 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 38390f7..4dfbb80 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1686,7 +1686,7 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
}
EXPORT_SYMBOL(intel_gmch_probe);
-const struct intel_gtt *intel_gtt_get(void)
+struct intel_gtt *intel_gtt_get(void)
{
return &intel_private.base;
}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ffbc915..eda983b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1492,19 +1492,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
goto free_priv;
}
- ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
- if (!ret) {
- DRM_ERROR("failed to set up gmch\n");
- ret = -EIO;
+ ret = i915_gem_gtt_init(dev);
+ if (ret)
goto put_bridge;
- }
-
- dev_priv->mm.gtt = intel_gtt_get();
- if (!dev_priv->mm.gtt) {
- DRM_ERROR("Failed to initialize GTT\n");
- ret = -ENODEV;
- goto put_gmch;
- }
i915_kick_out_firmware_fb(dev_priv);
@@ -1678,7 +1668,7 @@ out_mtrrfree:
out_rmmap:
pci_iounmap(dev->pdev, dev_priv->regs);
put_gmch:
- intel_gmch_remove();
+ i915_gem_gtt_fini(dev);
put_bridge:
pci_dev_put(dev_priv->bridge_dev);
free_priv:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bf628c4..534d282 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -687,7 +687,7 @@ typedef struct drm_i915_private {
struct {
/** Bridge to intel-gtt-ko */
- const struct intel_gtt *gtt;
+ struct intel_gtt *gtt;
/** Memory allocator for GTT stolen memory */
struct drm_mm stolen;
/** Memory allocator for GTT */
@@ -1507,6 +1507,14 @@ void i915_gem_init_global_gtt(struct drm_device *dev,
unsigned long start,
unsigned long mappable_end,
unsigned long end);
+int i915_gem_gtt_init(struct drm_device *dev);
+void i915_gem_gtt_fini(struct drm_device *dev);
+extern inline void i915_gem_chipset_flush(struct drm_device *dev)
+{
+ if (INTEL_INFO(dev)->gen < 6)
+ intel_gtt_chipset_flush();
+}
+
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct drm_device *dev, int min_size,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e04820c..ae3d4c1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -846,12 +846,12 @@ out:
* domain anymore. */
if (obj->base.write_domain != I915_GEM_DOMAIN_CPU) {
i915_gem_clflush_object(obj);
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
}
}
if (needs_clflush_after)
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
return ret;
}
@@ -3059,7 +3059,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj)
return;
i915_gem_clflush_object(obj);
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(obj->base.dev);
old_write_domain = obj->base.write_domain;
obj->base.write_domain = 0;
@@ -3960,7 +3960,7 @@ i915_gem_init_hw(struct drm_device *dev)
drm_i915_private_t *dev_priv = dev->dev_private;
int ret;
- if (!intel_enable_gtt())
+ if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
return -EIO;
if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1))
@@ -4295,7 +4295,7 @@ void i915_gem_detach_phys_object(struct drm_device *dev,
page_cache_release(page);
}
}
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
obj->phys_obj->cur_obj = NULL;
obj->phys_obj = NULL;
@@ -4382,7 +4382,7 @@ i915_gem_phys_pwrite(struct drm_device *dev,
return -EFAULT;
}
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 6a2f3e5..1af00b5 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -673,7 +673,7 @@ i915_gem_execbuffer_move_to_gpu(struct intel_ring_buffer *ring,
}
if (flush_domains & I915_GEM_DOMAIN_CPU)
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(ring->dev);
if (flush_domains & I915_GEM_DOMAIN_GTT)
wmb();
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 89c273f..96c8ef3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -305,13 +305,38 @@ static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible)
dev_priv->mm.interruptible = interruptible;
}
+
+static void i915_ggtt_clear_range(struct drm_device *dev,
+ unsigned first_entry,
+ unsigned num_entries)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ gtt_pte_t scratch_pte;
+ volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
+ const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
+
+ if (INTEL_INFO(dev)->gen < 6) {
+ intel_gtt_clear_range(first_entry, num_entries);
+ return;
+ }
+
+ if (WARN(num_entries > max_entries,
+ "First entry = %d; Num entries = %d (max=%d)\n",
+ first_entry, num_entries, max_entries))
+ num_entries = max_entries;
+
+ scratch_pte = pte_encode(dev, dev_priv->mm.gtt->scratch_page_dma, I915_CACHE_LLC);
+ memset_io(gtt_base, scratch_pte, num_entries * sizeof(scratch_pte));
+ readl(gtt_base);
+}
+
void i915_gem_restore_gtt_mappings(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj;
/* First fill our portion of the GTT with scratch pages */
- intel_gtt_clear_range(dev_priv->mm.gtt_start / PAGE_SIZE,
+ i915_ggtt_clear_range(dev, dev_priv->mm.gtt_start / PAGE_SIZE,
(dev_priv->mm.gtt_end - dev_priv->mm.gtt_start) / PAGE_SIZE);
list_for_each_entry(obj, &dev_priv->mm.bound_list, gtt_list) {
@@ -319,7 +344,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
i915_gem_gtt_bind_object(obj, obj->cache_level);
}
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
}
int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
@@ -335,21 +360,70 @@ int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
return 0;
}
+/*
+ * Binds an object into the global gtt with the specified cache level. The object
+ * will be accessible to the GPU via commands whose operands reference offsets
+ * within the global GTT as well as accessible by the GPU through the GMADR
+ * mapped BAR (dev_priv->mm.gtt->gtt).
+ */
+static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
+ enum i915_cache_level level)
+{
+ struct drm_device *dev = obj->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct sg_table *st = obj->pages;
+ struct scatterlist *sg = st->sgl;
+ const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
+ const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
+ gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gtt->gtt + first_entry;
+ int unused, i = 0;
+ unsigned int len, m = 0;
+ dma_addr_t addr;
+
+ for_each_sg(st->sgl, sg, st->nents, unused) {
+ len = sg_dma_len(sg) >> PAGE_SHIFT;
+ for (m = 0; m < len; m++) {
+ addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
+ gtt_entries[i] = pte_encode(dev, addr, level);
+ i++;
+ if (WARN_ON(i > max_entries))
+ goto out;
+ }
+ }
+
+out:
+ if (WARN(i == 0, "No ptes updated for %p\n", obj))
+ return;
+
+ /* XXX: This serves as a posting read to make sure that the PTE has
+ * actually been updated. There is some concern that even though
+ * registers and PTEs are within the same BAR that they are potentially
+ * of NUMA access patterns. Therefore, even with the way we assume
+ * hardware should work, we must keep this posting read for paranoia.
+ */
+ WARN_ON(readl(>t_entries[i-1]) != pte_encode(dev, addr, level));
+}
+
void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
enum i915_cache_level cache_level)
{
struct drm_device *dev = obj->base.dev;
- unsigned int agp_type = cache_level_to_agp_type(dev, cache_level);
+ if (INTEL_INFO(dev)->gen < 6) {
+ unsigned int agp_type = cache_level_to_agp_type(dev, cache_level);
+ intel_gtt_insert_sg_entries(obj->pages,
+ obj->gtt_space->start >> PAGE_SHIFT,
+ agp_type);
+ } else {
+ gen6_ggtt_bind_object(obj, cache_level);
+ }
- intel_gtt_insert_sg_entries(obj->pages,
- obj->gtt_space->start >> PAGE_SHIFT,
- agp_type);
obj->has_global_gtt_mapping = 1;
}
void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
{
- intel_gtt_clear_range(obj->gtt_space->start >> PAGE_SHIFT,
+ i915_ggtt_clear_range(obj->base.dev,
+ obj->gtt_space->start >> PAGE_SHIFT,
obj->base.size >> PAGE_SHIFT);
obj->has_global_gtt_mapping = 0;
@@ -407,5 +481,129 @@ void i915_gem_init_global_gtt(struct drm_device *dev,
dev_priv->mm.mappable_gtt_total = min(end, mappable_end) - start;
/* ... but ensure that we clear the entire range. */
- intel_gtt_clear_range(start / PAGE_SIZE, (end-start) / PAGE_SIZE);
+ i915_ggtt_clear_range(dev, start / PAGE_SIZE, (end-start) / PAGE_SIZE);
+}
+
+static int setup_scratch_page(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct page *page;
+ dma_addr_t dma_addr;
+
+ page = alloc_page(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO);
+ if (page == NULL)
+ return -ENOMEM;
+ get_page(page);
+ set_pages_uc(page, 1);
+
+#ifdef CONFIG_INTEL_IOMMU
+ dma_addr = pci_map_page(dev->pdev, page, 0, PAGE_SIZE,
+ PCI_DMA_BIDIRECTIONAL);
+ if (pci_dma_mapping_error(dev->pdev, dma_addr))
+ return -EINVAL;
+#else
+ dma_addr = page_to_phys(page);
+#endif
+ dev_priv->mm.gtt->scratch_page = page;
+ dev_priv->mm.gtt->scratch_page_dma = dma_addr;
+
+ return 0;
+}
+
+static void teardown_scratch_page(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ set_pages_wb(dev_priv->mm.gtt->scratch_page, 1);
+ pci_unmap_page(dev->pdev, dev_priv->mm.gtt->scratch_page_dma,
+ PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
+ put_page(dev_priv->mm.gtt->scratch_page);
+ __free_page(dev_priv->mm.gtt->scratch_page);
+}
+
+static inline unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
+{
+ snb_gmch_ctl >>= SNB_GMCH_GGMS_SHIFT;
+ snb_gmch_ctl &= SNB_GMCH_GGMS_MASK;
+ return snb_gmch_ctl << 20;
+}
+
+static inline unsigned int gen6_get_stolen_size(u16 snb_gmch_ctl)
+{
+ snb_gmch_ctl >>= SNB_GMCH_GMS_SHIFT;
+ snb_gmch_ctl &= SNB_GMCH_GMS_MASK;
+ return snb_gmch_ctl << 25; /* 32 MB units */
+}
+
+int i915_gem_gtt_init(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ phys_addr_t gtt_bus_addr;
+ u16 snb_gmch_ctl;
+ u32 tmp;
+ int ret;
+
+ /* On modern platforms we need not worry ourself with the legacy
+ * hostbridge query stuff. Skip it entirely
+ */
+ if (INTEL_INFO(dev)->gen < 6) {
+ ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
+ if (!ret) {
+ DRM_ERROR("failed to set up gmch\n");
+ return -EIO;
+ }
+
+ dev_priv->mm.gtt = intel_gtt_get();
+ if (!dev_priv->mm.gtt) {
+ DRM_ERROR("Failed to initialize GTT\n");
+ intel_gmch_remove();
+ return -ENODEV;
+ }
+ return 0;
+ }
+
+
+ dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
+ if (!dev_priv->mm.gtt)
+ return -ENOMEM;
+
+ if (!pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(40)))
+ pci_set_consistent_dma_mask(dev->pdev, DMA_BIT_MASK(40));
+
+ pci_read_config_dword(dev->pdev, PCI_BASE_ADDRESS_0, &tmp);
+ /* For GEN6+ the PTEs for the ggtt live at 2MB + BAR0 */
+ gtt_bus_addr = (tmp & PCI_BASE_ADDRESS_MEM_MASK) + (2<<20);
+
+ dev_priv->mm.gtt->gtt_mappable_entries = pci_resource_len(dev->pdev, 2) >> PAGE_SHIFT;
+ pci_read_config_dword(dev->pdev, PCI_BASE_ADDRESS_2, &tmp);
+ dev_priv->mm.gtt->gma_bus_addr = tmp & PCI_BASE_ADDRESS_MEM_MASK;
+
+ /* i9xx_setup */
+ pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
+ dev_priv->mm.gtt->gtt_total_entries =
+ gen6_get_total_gtt_size(snb_gmch_ctl) / sizeof(gtt_pte_t);
+ dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
+
+ ret = setup_scratch_page(dev);
+ if (ret)
+ return ret;
+
+ dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
+ dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
+
+ /* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
+ DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
+ DRM_DEBUG_DRIVER("GTT total entries = %dK\n", dev_priv->mm.gtt->gtt_total_entries >> 10);
+ DRM_DEBUG_DRIVER("GTT stolen size = %dM\n", dev_priv->mm.gtt->stolen_size >> 20);
+
+ return 0;
+}
+
+void i915_gem_gtt_fini(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ iounmap(dev_priv->mm.gtt->gtt);
+ teardown_scratch_page(dev);
+ if (INTEL_INFO(dev)->gen < 6)
+ intel_gmch_remove();
+ kfree(dev_priv->mm.gtt);
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f4d70b0..10a6e9b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -40,6 +40,12 @@
*/
#define INTEL_GMCH_CTRL 0x52
#define INTEL_GMCH_VGA_DISABLE (1 << 1)
+#define SNB_GMCH_CTRL 0x50
+#define SNB_GMCH_GGMS_SHIFT 8 /* GTT Graphics Memory Size */
+#define SNB_GMCH_GGMS_MASK 0x3
+#define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */
+#define SNB_GMCH_GMS_MASK 0x1f
+
/* PCI config space */
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 2e37e9f..94e8f2c 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -3,7 +3,7 @@
#ifndef _DRM_INTEL_GTT_H
#define _DRM_INTEL_GTT_H
-const struct intel_gtt {
+struct intel_gtt {
/* Size of memory reserved for graphics by the BIOS */
unsigned int stolen_size;
/* Total number of gtt entries. */
@@ -17,6 +17,7 @@ const struct intel_gtt {
unsigned int do_idle_maps : 1;
/* Share the scratch page dma with ppgtts. */
dma_addr_t scratch_page_dma;
+ struct page *scratch_page;
/* for ppgtt PDE access */
u32 __iomem *gtt;
/* needed for ioremap in drm/i915 */
--
1.8.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH 6/9 v3] drm/i915: Stop using AGP layer for GEN6+
2012-10-26 20:54 ` [PATCH 06/10 v2] " Ben Widawsky
@ 2012-10-29 2:08 ` Ben Widawsky
2012-10-31 9:57 ` Daniel Vetter
2012-10-31 10:37 ` [PATCH 06/10 v2] " Chris Wilson
1 sibling, 1 reply; 50+ messages in thread
From: Ben Widawsky @ 2012-10-29 2:08 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
As a quick hack we make the old intel_gtt structure mutable so we can
fool a bunch of the existing code which depends on elements in that data
structure. We can/should try to remove this in a subsequent patch.
This should preserve the old gtt init behavior which upon writing these
patches seems incorrect. The next patch will fix these things.
The one exception is VLV which doesn't have the preserved flush control
write behavior. Since we want to do that for all GEN6+ stuff, we'll
handle that in a later patch. Mainstream VLV support doesn't actually
exist yet anyway.
v2: Update the comment to remove the "voodoo"
Check that the last pte written matches what we readback
v3: actually kill cache_level_to_agp_type since most of the flags will
dissapear in an upcoming patch
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/char/agp/intel-gtt.c | 2 +-
drivers/gpu/drm/i915/i915_dma.c | 16 +-
drivers/gpu/drm/i915/i915_drv.h | 10 +-
drivers/gpu/drm/i915/i915_gem.c | 12 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 233 +++++++++++++++++++++++++----
drivers/gpu/drm/i915/i915_reg.h | 6 +
include/drm/intel-gtt.h | 3 +-
8 files changed, 233 insertions(+), 51 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 38390f7..4dfbb80 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1686,7 +1686,7 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
}
EXPORT_SYMBOL(intel_gmch_probe);
-const struct intel_gtt *intel_gtt_get(void)
+struct intel_gtt *intel_gtt_get(void)
{
return &intel_private.base;
}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index d04facb..d9b4a49 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1494,19 +1494,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
goto free_priv;
}
- ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
- if (!ret) {
- DRM_ERROR("failed to set up gmch\n");
- ret = -EIO;
+ ret = i915_gem_gtt_init(dev);
+ if (ret)
goto put_bridge;
- }
-
- dev_priv->mm.gtt = intel_gtt_get();
- if (!dev_priv->mm.gtt) {
- DRM_ERROR("Failed to initialize GTT\n");
- ret = -ENODEV;
- goto put_gmch;
- }
i915_kick_out_firmware_fb(dev_priv);
@@ -1680,7 +1670,7 @@ out_mtrrfree:
out_rmmap:
pci_iounmap(dev->pdev, dev_priv->regs);
put_gmch:
- intel_gmch_remove();
+ i915_gem_gtt_fini(dev);
put_bridge:
pci_dev_put(dev_priv->bridge_dev);
free_priv:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 43d295a..9aa4512 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -685,7 +685,7 @@ typedef struct drm_i915_private {
struct {
/** Bridge to intel-gtt-ko */
- const struct intel_gtt *gtt;
+ struct intel_gtt *gtt;
/** Memory allocator for GTT stolen memory */
struct drm_mm stolen;
/** Memory allocator for GTT */
@@ -1504,6 +1504,14 @@ void i915_gem_init_global_gtt(struct drm_device *dev,
unsigned long start,
unsigned long mappable_end,
unsigned long end);
+int i915_gem_gtt_init(struct drm_device *dev);
+void i915_gem_gtt_fini(struct drm_device *dev);
+extern inline void i915_gem_chipset_flush(struct drm_device *dev)
+{
+ if (INTEL_INFO(dev)->gen < 6)
+ intel_gtt_chipset_flush();
+}
+
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct drm_device *dev, int min_size,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7dd1034..859ac4f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -845,12 +845,12 @@ out:
* domain anymore. */
if (obj->base.write_domain != I915_GEM_DOMAIN_CPU) {
i915_gem_clflush_object(obj);
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
}
}
if (needs_clflush_after)
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
return ret;
}
@@ -3058,7 +3058,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj)
return;
i915_gem_clflush_object(obj);
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(obj->base.dev);
old_write_domain = obj->base.write_domain;
obj->base.write_domain = 0;
@@ -3959,7 +3959,7 @@ i915_gem_init_hw(struct drm_device *dev)
drm_i915_private_t *dev_priv = dev->dev_private;
int ret;
- if (!intel_enable_gtt())
+ if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
return -EIO;
if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1))
@@ -4294,7 +4294,7 @@ void i915_gem_detach_phys_object(struct drm_device *dev,
page_cache_release(page);
}
}
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
obj->phys_obj->cur_obj = NULL;
obj->phys_obj = NULL;
@@ -4381,7 +4381,7 @@ i915_gem_phys_pwrite(struct drm_device *dev,
return -EFAULT;
}
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 91d43d5..d80e9dd 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -672,7 +672,7 @@ i915_gem_execbuffer_move_to_gpu(struct intel_ring_buffer *ring,
}
if (flush_domains & I915_GEM_DOMAIN_CPU)
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(ring->dev);
if (flush_domains & I915_GEM_DOMAIN_GTT)
wmb();
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 06202fd..98367a5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -262,26 +262,6 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
obj->base.size >> PAGE_SHIFT);
}
-/* XXX kill agp_type! */
-static unsigned int cache_level_to_agp_type(struct drm_device *dev,
- enum i915_cache_level cache_level)
-{
- switch (cache_level) {
- case I915_CACHE_LLC_MLC:
- /* Older chipsets do not have this extra level of CPU
- * cacheing, so fallthrough and request the PTE simply
- * as cached.
- */
- if (INTEL_INFO(dev)->gen >= 6 && !IS_HASWELL(dev))
- return AGP_USER_CACHED_MEMORY_LLC_MLC;
- case I915_CACHE_LLC:
- return AGP_USER_CACHED_MEMORY;
- default:
- case I915_CACHE_NONE:
- return AGP_USER_MEMORY;
- }
-}
-
static bool do_idling(struct drm_i915_private *dev_priv)
{
bool ret = dev_priv->mm.interruptible;
@@ -304,13 +284,38 @@ static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible)
dev_priv->mm.interruptible = interruptible;
}
+
+static void i915_ggtt_clear_range(struct drm_device *dev,
+ unsigned first_entry,
+ unsigned num_entries)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ gtt_pte_t scratch_pte;
+ volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
+ const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
+
+ if (INTEL_INFO(dev)->gen < 6) {
+ intel_gtt_clear_range(first_entry, num_entries);
+ return;
+ }
+
+ if (WARN(num_entries > max_entries,
+ "First entry = %d; Num entries = %d (max=%d)\n",
+ first_entry, num_entries, max_entries))
+ num_entries = max_entries;
+
+ scratch_pte = pte_encode(dev, dev_priv->mm.gtt->scratch_page_dma, I915_CACHE_LLC);
+ memset_io(gtt_base, scratch_pte, num_entries * sizeof(scratch_pte));
+ readl(gtt_base);
+}
+
void i915_gem_restore_gtt_mappings(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj;
/* First fill our portion of the GTT with scratch pages */
- intel_gtt_clear_range(dev_priv->mm.gtt_start / PAGE_SIZE,
+ i915_ggtt_clear_range(dev, dev_priv->mm.gtt_start / PAGE_SIZE,
(dev_priv->mm.gtt_end - dev_priv->mm.gtt_start) / PAGE_SIZE);
list_for_each_entry(obj, &dev_priv->mm.bound_list, gtt_list) {
@@ -318,7 +323,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
i915_gem_gtt_bind_object(obj, obj->cache_level);
}
- intel_gtt_chipset_flush();
+ i915_gem_chipset_flush(dev);
}
int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
@@ -334,21 +339,69 @@ int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
return 0;
}
+/*
+ * Binds an object into the global gtt with the specified cache level. The object
+ * will be accessible to the GPU via commands whose operands reference offsets
+ * within the global GTT as well as accessible by the GPU through the GMADR
+ * mapped BAR (dev_priv->mm.gtt->gtt).
+ */
+static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
+ enum i915_cache_level level)
+{
+ struct drm_device *dev = obj->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct sg_table *st = obj->pages;
+ struct scatterlist *sg = st->sgl;
+ const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
+ const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
+ gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gtt->gtt + first_entry;
+ int unused, i = 0;
+ unsigned int len, m = 0;
+ dma_addr_t addr;
+
+ for_each_sg(st->sgl, sg, st->nents, unused) {
+ len = sg_dma_len(sg) >> PAGE_SHIFT;
+ for (m = 0; m < len; m++) {
+ addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
+ gtt_entries[i] = pte_encode(dev, addr, level);
+ i++;
+ if (WARN_ON(i > max_entries))
+ goto out;
+ }
+ }
+
+out:
+ if (WARN(i == 0, "No ptes updated for %p\n", obj))
+ return;
+
+ /* XXX: This serves as a posting read to make sure that the PTE has
+ * actually been updated. There is some concern that even though
+ * registers and PTEs are within the same BAR that they are potentially
+ * of NUMA access patterns. Therefore, even with the way we assume
+ * hardware should work, we must keep this posting read for paranoia.
+ */
+ WARN_ON(readl(>t_entries[i-1]) != pte_encode(dev, addr, level));
+}
+
void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
enum i915_cache_level cache_level)
{
struct drm_device *dev = obj->base.dev;
- unsigned int agp_type = cache_level_to_agp_type(dev, cache_level);
+ if (INTEL_INFO(dev)->gen < 6) {
+ intel_gtt_insert_sg_entries(obj->pages,
+ obj->gtt_space->start >> PAGE_SHIFT,
+ AGP_USER_MEMORY);
+ } else {
+ gen6_ggtt_bind_object(obj, cache_level);
+ }
- intel_gtt_insert_sg_entries(obj->pages,
- obj->gtt_space->start >> PAGE_SHIFT,
- agp_type);
obj->has_global_gtt_mapping = 1;
}
void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
{
- intel_gtt_clear_range(obj->gtt_space->start >> PAGE_SHIFT,
+ i915_ggtt_clear_range(obj->base.dev,
+ obj->gtt_space->start >> PAGE_SHIFT,
obj->base.size >> PAGE_SHIFT);
obj->has_global_gtt_mapping = 0;
@@ -406,5 +459,129 @@ void i915_gem_init_global_gtt(struct drm_device *dev,
dev_priv->mm.mappable_gtt_total = min(end, mappable_end) - start;
/* ... but ensure that we clear the entire range. */
- intel_gtt_clear_range(start / PAGE_SIZE, (end-start) / PAGE_SIZE);
+ i915_ggtt_clear_range(dev, start / PAGE_SIZE, (end-start) / PAGE_SIZE);
+}
+
+static int setup_scratch_page(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct page *page;
+ dma_addr_t dma_addr;
+
+ page = alloc_page(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO);
+ if (page == NULL)
+ return -ENOMEM;
+ get_page(page);
+ set_pages_uc(page, 1);
+
+#ifdef CONFIG_INTEL_IOMMU
+ dma_addr = pci_map_page(dev->pdev, page, 0, PAGE_SIZE,
+ PCI_DMA_BIDIRECTIONAL);
+ if (pci_dma_mapping_error(dev->pdev, dma_addr))
+ return -EINVAL;
+#else
+ dma_addr = page_to_phys(page);
+#endif
+ dev_priv->mm.gtt->scratch_page = page;
+ dev_priv->mm.gtt->scratch_page_dma = dma_addr;
+
+ return 0;
+}
+
+static void teardown_scratch_page(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ set_pages_wb(dev_priv->mm.gtt->scratch_page, 1);
+ pci_unmap_page(dev->pdev, dev_priv->mm.gtt->scratch_page_dma,
+ PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
+ put_page(dev_priv->mm.gtt->scratch_page);
+ __free_page(dev_priv->mm.gtt->scratch_page);
+}
+
+static inline unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
+{
+ snb_gmch_ctl >>= SNB_GMCH_GGMS_SHIFT;
+ snb_gmch_ctl &= SNB_GMCH_GGMS_MASK;
+ return snb_gmch_ctl << 20;
+}
+
+static inline unsigned int gen6_get_stolen_size(u16 snb_gmch_ctl)
+{
+ snb_gmch_ctl >>= SNB_GMCH_GMS_SHIFT;
+ snb_gmch_ctl &= SNB_GMCH_GMS_MASK;
+ return snb_gmch_ctl << 25; /* 32 MB units */
+}
+
+int i915_gem_gtt_init(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ phys_addr_t gtt_bus_addr;
+ u16 snb_gmch_ctl;
+ u32 tmp;
+ int ret;
+
+ /* On modern platforms we need not worry ourself with the legacy
+ * hostbridge query stuff. Skip it entirely
+ */
+ if (INTEL_INFO(dev)->gen < 6) {
+ ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
+ if (!ret) {
+ DRM_ERROR("failed to set up gmch\n");
+ return -EIO;
+ }
+
+ dev_priv->mm.gtt = intel_gtt_get();
+ if (!dev_priv->mm.gtt) {
+ DRM_ERROR("Failed to initialize GTT\n");
+ intel_gmch_remove();
+ return -ENODEV;
+ }
+ return 0;
+ }
+
+
+ dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
+ if (!dev_priv->mm.gtt)
+ return -ENOMEM;
+
+ if (!pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(40)))
+ pci_set_consistent_dma_mask(dev->pdev, DMA_BIT_MASK(40));
+
+ pci_read_config_dword(dev->pdev, PCI_BASE_ADDRESS_0, &tmp);
+ /* For GEN6+ the PTEs for the ggtt live at 2MB + BAR0 */
+ gtt_bus_addr = (tmp & PCI_BASE_ADDRESS_MEM_MASK) + (2<<20);
+
+ dev_priv->mm.gtt->gtt_mappable_entries = pci_resource_len(dev->pdev, 2) >> PAGE_SHIFT;
+ pci_read_config_dword(dev->pdev, PCI_BASE_ADDRESS_2, &tmp);
+ dev_priv->mm.gtt->gma_bus_addr = tmp & PCI_BASE_ADDRESS_MEM_MASK;
+
+ /* i9xx_setup */
+ pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
+ dev_priv->mm.gtt->gtt_total_entries =
+ gen6_get_total_gtt_size(snb_gmch_ctl) / sizeof(gtt_pte_t);
+ dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
+
+ ret = setup_scratch_page(dev);
+ if (ret)
+ return ret;
+
+ dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
+ dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
+
+ /* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
+ DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
+ DRM_DEBUG_DRIVER("GTT total entries = %dK\n", dev_priv->mm.gtt->gtt_total_entries >> 10);
+ DRM_DEBUG_DRIVER("GTT stolen size = %dM\n", dev_priv->mm.gtt->stolen_size >> 20);
+
+ return 0;
+}
+
+void i915_gem_gtt_fini(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ iounmap(dev_priv->mm.gtt->gtt);
+ teardown_scratch_page(dev);
+ if (INTEL_INFO(dev)->gen < 6)
+ intel_gmch_remove();
+ kfree(dev_priv->mm.gtt);
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index be22aeb..2b31f63 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -41,6 +41,12 @@
*/
#define INTEL_GMCH_CTRL 0x52
#define INTEL_GMCH_VGA_DISABLE (1 << 1)
+#define SNB_GMCH_CTRL 0x50
+#define SNB_GMCH_GGMS_SHIFT 8 /* GTT Graphics Memory Size */
+#define SNB_GMCH_GGMS_MASK 0x3
+#define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */
+#define SNB_GMCH_GMS_MASK 0x1f
+
/* PCI config space */
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 2e37e9f..94e8f2c 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -3,7 +3,7 @@
#ifndef _DRM_INTEL_GTT_H
#define _DRM_INTEL_GTT_H
-const struct intel_gtt {
+struct intel_gtt {
/* Size of memory reserved for graphics by the BIOS */
unsigned int stolen_size;
/* Total number of gtt entries. */
@@ -17,6 +17,7 @@ const struct intel_gtt {
unsigned int do_idle_maps : 1;
/* Share the scratch page dma with ppgtts. */
dma_addr_t scratch_page_dma;
+ struct page *scratch_page;
/* for ppgtt PDE access */
u32 __iomem *gtt;
/* needed for ioremap in drm/i915 */
--
1.8.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 6/9 v3] drm/i915: Stop using AGP layer for GEN6+
2012-10-29 2:08 ` [PATCH 6/9 v3] " Ben Widawsky
@ 2012-10-31 9:57 ` Daniel Vetter
2012-10-31 15:51 ` Ben Widawsky
0 siblings, 1 reply; 50+ messages in thread
From: Daniel Vetter @ 2012-10-31 9:57 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Sun, Oct 28, 2012 at 07:08:56PM -0700, Ben Widawsky wrote:
> As a quick hack we make the old intel_gtt structure mutable so we can
> fool a bunch of the existing code which depends on elements in that data
> structure. We can/should try to remove this in a subsequent patch.
>
> This should preserve the old gtt init behavior which upon writing these
> patches seems incorrect. The next patch will fix these things.
>
> The one exception is VLV which doesn't have the preserved flush control
> write behavior. Since we want to do that for all GEN6+ stuff, we'll
> handle that in a later patch. Mainstream VLV support doesn't actually
> exist yet anyway.
>
> v2: Update the comment to remove the "voodoo"
> Check that the last pte written matches what we readback
>
> v3: actually kill cache_level_to_agp_type since most of the flags will
> dissapear in an upcoming patch
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Just wanted to merge this entire pile, but then stumbled over a small
thing: You can't kill the cache_level_to_agp_type stuff, since we now have
different cache levels even on pre-gen6. Otherwise you'll break cacheable
bos in SNA, which will anger Chris ;-)
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 6/9 v3] drm/i915: Stop using AGP layer for GEN6+
2012-10-31 9:57 ` Daniel Vetter
@ 2012-10-31 15:51 ` Ben Widawsky
0 siblings, 0 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-31 15:51 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, 31 Oct 2012 10:57:08 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sun, Oct 28, 2012 at 07:08:56PM -0700, Ben Widawsky wrote:
> > As a quick hack we make the old intel_gtt structure mutable so we
> > can fool a bunch of the existing code which depends on elements in
> > that data structure. We can/should try to remove this in a
> > subsequent patch.
> >
> > This should preserve the old gtt init behavior which upon writing
> > these patches seems incorrect. The next patch will fix these things.
> >
> > The one exception is VLV which doesn't have the preserved flush
> > control write behavior. Since we want to do that for all GEN6+
> > stuff, we'll handle that in a later patch. Mainstream VLV support
> > doesn't actually exist yet anyway.
> >
> > v2: Update the comment to remove the "voodoo"
> > Check that the last pte written matches what we readback
> >
> > v3: actually kill cache_level_to_agp_type since most of the flags
> > will dissapear in an upcoming patch
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>
> Just wanted to merge this entire pile, but then stumbled over a small
> thing: You can't kill the cache_level_to_agp_type stuff, since we now
> have different cache levels even on pre-gen6. Otherwise you'll break
> cacheable bos in SNA, which will anger Chris ;-)
>
> Thanks, Daniel
Well, I'm not even going to go bother to figure out what
I830_PTE_SYSTEM_CACHED even means. I can still kill it and turn
it into a simple if statement in bind_object.
Chris left some other comments to address, so it needed a new version
anyway.
Thanks for the feedback.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 06/10 v2] drm/i915: Stop using AGP layer for GEN6+
2012-10-26 20:54 ` [PATCH 06/10 v2] " Ben Widawsky
2012-10-29 2:08 ` [PATCH 6/9 v3] " Ben Widawsky
@ 2012-10-31 10:37 ` Chris Wilson
1 sibling, 0 replies; 50+ messages in thread
From: Chris Wilson @ 2012-10-31 10:37 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Fri, 26 Oct 2012 13:54:24 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> As a quick hack we make the old intel_gtt structure mutable so we can
> fool a bunch of the existing code which depends on elements in that data
> structure. We can/should try to remove this in a subsequent patch.
>
> This should preserve the old gtt init behavior which upon writing these
> patches seems incorrect. The next patch will fix these things.
>
> The one exception is VLV which doesn't have the preserved flush control
> write behavior. Since we want to do that for all GEN6+ stuff, we'll
> handle that in a later patch. Mainstream VLV support doesn't actually
> exist yet anyway.
>
> v2: Update the comment to remove the "voodoo"
> Check that the last pte written matches what we readback
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> +static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
> + enum i915_cache_level level)
> +{
> + struct drm_device *dev = obj->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct sg_table *st = obj->pages;
> + struct scatterlist *sg = st->sgl;
> + const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
> + const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
> + gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gtt->gtt + first_entry;
> + int unused, i = 0;
> + unsigned int len, m = 0;
> + dma_addr_t addr;
> +
> + for_each_sg(st->sgl, sg, st->nents, unused) {
> + len = sg_dma_len(sg) >> PAGE_SHIFT;
> + for (m = 0; m < len; m++) {
> + addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
> + gtt_entries[i] = pte_encode(dev, addr, level);
> + i++;
> + if (WARN_ON(i > max_entries))
> + goto out;
> + }
> + }
In the clear range above, your detection of a programming error is much
better. And this is truly a BUG_ON() event because state is then
completely screwed up.
> +int i915_gem_gtt_init(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + phys_addr_t gtt_bus_addr;
> + u16 snb_gmch_ctl;
> + u32 tmp;
> + int ret;
> +
> + /* On modern platforms we need not worry ourself with the legacy
> + * hostbridge query stuff. Skip it entirely
> + */
> + if (INTEL_INFO(dev)->gen < 6) {
> + ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
> + if (!ret) {
> + DRM_ERROR("failed to set up gmch\n");
> + return -EIO;
> + }
> +
> + dev_priv->mm.gtt = intel_gtt_get();
> + if (!dev_priv->mm.gtt) {
> + DRM_ERROR("Failed to initialize GTT\n");
> + intel_gmch_remove();
> + return -ENODEV;
> + }
> + return 0;
> + }
> +
> +
Two blank lines for the price of one. You really like to separate
yourself from the pre-HSW legacy...
> + dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
> + if (!dev_priv->mm.gtt)
> + return -ENOMEM;
> +
> + if (!pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(40)))
> + pci_set_consistent_dma_mask(dev->pdev, DMA_BIT_MASK(40));
> +
> + pci_read_config_dword(dev->pdev, PCI_BASE_ADDRESS_0, &tmp);
> + /* For GEN6+ the PTEs for the ggtt live at 2MB + BAR0 */
> + gtt_bus_addr = (tmp & PCI_BASE_ADDRESS_MEM_MASK) + (2<<20);
> +
> + dev_priv->mm.gtt->gtt_mappable_entries = pci_resource_len(dev->pdev, 2) >> PAGE_SHIFT;
Probably best to check the value here and throw a paranoid tantrum in
case it is too small or too large.
> + pci_read_config_dword(dev->pdev, PCI_BASE_ADDRESS_2, &tmp);
> + dev_priv->mm.gtt->gma_bus_addr = tmp & PCI_BASE_ADDRESS_MEM_MASK;
> +
> + /* i9xx_setup */
> + pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
> + dev_priv->mm.gtt->gtt_total_entries =
> + gen6_get_total_gtt_size(snb_gmch_ctl) / sizeof(gtt_pte_t);
> + dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
> +
> + ret = setup_scratch_page(dev);
> + if (ret)
> + return ret;
Missing cleanup.
> +
> + dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
> + dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
Can fail.
> + /* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
> + DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
> + DRM_DEBUG_DRIVER("GTT total entries = %dK\n", dev_priv->mm.gtt->gtt_total_entries >> 10);
> + DRM_DEBUG_DRIVER("GTT stolen size = %dM\n", dev_priv->mm.gtt->stolen_size >> 20);
I think it would be nice for our users to have a one line INFO summarizing
the memory availabe to the graphics device.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 07/10] drm/i915: Calculate correct stolen size for GEN7+
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
` (5 preceding siblings ...)
2012-10-23 1:34 ` [PATCH 06/10] drm/i915: Stop using AGP layer for GEN6+ Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-25 21:06 ` Jesse Barnes
2012-10-23 1:34 ` [PATCH 08/10] drm/i915: Kill off now unused gen6+ AGP code Ben Widawsky
` (3 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
This bug existed in the old code, but was easier to fix here in the
rework. Unfortunately gen7 doesn't have a nice way to figure out the
size and we must use a lookup table.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
drivers/gpu/drm/i915/i915_reg.h | 2 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7d3ec42..16fe960 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -528,6 +528,15 @@ static inline unsigned int gen6_get_stolen_size(u16 snb_gmch_ctl)
return snb_gmch_ctl << 25; /* 32 MB units */
}
+static inline unsigned int gen7_get_stolen_size(u16 snb_gmch_ctl)
+{
+ static const int stolen_decoder[] = {
+ 0, 0, 0, 0, 0, 32, 48, 64, 128, 256, 96, 160, 224, 352};
+ snb_gmch_ctl >>= IVB_GMCH_GMS_SHIFT;
+ snb_gmch_ctl &= IVB_GMCH_GMS_MASK;
+ return stolen_decoder[snb_gmch_ctl] << 20;
+}
+
int i915_gem_gtt_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -575,7 +584,10 @@ int i915_gem_gtt_init(struct drm_device *dev)
pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
dev_priv->mm.gtt->gtt_total_entries =
gen6_get_total_gtt_size(snb_gmch_ctl) / sizeof(gtt_pte_t);
- dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
+ if (INTEL_INFO(dev)->gen < 7)
+ dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
+ else
+ dev_priv->mm.gtt->stolen_size = gen7_get_stolen_size(snb_gmch_ctl);
ret = setup_scratch_page(dev);
if (ret)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 10a6e9b..1d54328 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -45,6 +45,8 @@
#define SNB_GMCH_GGMS_MASK 0x3
#define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */
#define SNB_GMCH_GMS_MASK 0x1f
+#define IVB_GMCH_GMS_SHIFT 4
+#define IVB_GMCH_GMS_MASK 0xf
/* PCI config space */
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 07/10] drm/i915: Calculate correct stolen size for GEN7+
2012-10-23 1:34 ` [PATCH 07/10] drm/i915: Calculate correct stolen size for GEN7+ Ben Widawsky
@ 2012-10-25 21:06 ` Jesse Barnes
2012-10-25 22:15 ` Ben Widawsky
0 siblings, 1 reply; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 21:06 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:12 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> This bug existed in the old code, but was easier to fix here in the
> rework. Unfortunately gen7 doesn't have a nice way to figure out the
> size and we must use a lookup table.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7d3ec42..16fe960 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -528,6 +528,15 @@ static inline unsigned int gen6_get_stolen_size(u16 snb_gmch_ctl)
> return snb_gmch_ctl << 25; /* 32 MB units */
> }
>
> +static inline unsigned int gen7_get_stolen_size(u16 snb_gmch_ctl)
> +{
> + static const int stolen_decoder[] = {
> + 0, 0, 0, 0, 0, 32, 48, 64, 128, 256, 96, 160, 224, 352};
> + snb_gmch_ctl >>= IVB_GMCH_GMS_SHIFT;
> + snb_gmch_ctl &= IVB_GMCH_GMS_MASK;
> + return stolen_decoder[snb_gmch_ctl] << 20;
> +}
> +
> int i915_gem_gtt_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -575,7 +584,10 @@ int i915_gem_gtt_init(struct drm_device *dev)
> pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
> dev_priv->mm.gtt->gtt_total_entries =
> gen6_get_total_gtt_size(snb_gmch_ctl) / sizeof(gtt_pte_t);
> - dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
> + if (INTEL_INFO(dev)->gen < 7)
> + dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
> + else
> + dev_priv->mm.gtt->stolen_size = gen7_get_stolen_size(snb_gmch_ctl);
>
> ret = setup_scratch_page(dev);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 10a6e9b..1d54328 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -45,6 +45,8 @@
> #define SNB_GMCH_GGMS_MASK 0x3
> #define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */
> #define SNB_GMCH_GMS_MASK 0x1f
> +#define IVB_GMCH_GMS_SHIFT 4
> +#define IVB_GMCH_GMS_MASK 0xf
>
>
> /* PCI config space */
Hm my docs say different. Supposedly the size field starts at bit 3
and is 5 bits wide, going up to 512M. So just like SNB but with a
bigger max.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 07/10] drm/i915: Calculate correct stolen size for GEN7+
2012-10-25 21:06 ` Jesse Barnes
@ 2012-10-25 22:15 ` Ben Widawsky
2012-10-25 22:39 ` Jesse Barnes
0 siblings, 1 reply; 50+ messages in thread
From: Ben Widawsky @ 2012-10-25 22:15 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Thu, 25 Oct 2012 14:06:48 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 22 Oct 2012 18:34:12 -0700
> Ben Widawsky <ben@bwidawsk.net> wrote:
>
> > This bug existed in the old code, but was easier to fix here in the
> > rework. Unfortunately gen7 doesn't have a nice way to figure out the
> > size and we must use a lookup table.
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
> > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > 2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 7d3ec42..16fe960 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -528,6 +528,15 @@ static inline unsigned int gen6_get_stolen_size(u16 snb_gmch_ctl)
> > return snb_gmch_ctl << 25; /* 32 MB units */
> > }
> >
> > +static inline unsigned int gen7_get_stolen_size(u16 snb_gmch_ctl)
> > +{
> > + static const int stolen_decoder[] = {
> > + 0, 0, 0, 0, 0, 32, 48, 64, 128, 256, 96, 160, 224, 352};
> > + snb_gmch_ctl >>= IVB_GMCH_GMS_SHIFT;
> > + snb_gmch_ctl &= IVB_GMCH_GMS_MASK;
> > + return stolen_decoder[snb_gmch_ctl] << 20;
> > +}
> > +
> > int i915_gem_gtt_init(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -575,7 +584,10 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
> > dev_priv->mm.gtt->gtt_total_entries =
> > gen6_get_total_gtt_size(snb_gmch_ctl) / sizeof(gtt_pte_t);
> > - dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
> > + if (INTEL_INFO(dev)->gen < 7)
> > + dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
> > + else
> > + dev_priv->mm.gtt->stolen_size = gen7_get_stolen_size(snb_gmch_ctl);
> >
> > ret = setup_scratch_page(dev);
> > if (ret)
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 10a6e9b..1d54328 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -45,6 +45,8 @@
> > #define SNB_GMCH_GGMS_MASK 0x3
> > #define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */
> > #define SNB_GMCH_GMS_MASK 0x1f
> > +#define IVB_GMCH_GMS_SHIFT 4
> > +#define IVB_GMCH_GMS_MASK 0xf
> >
> >
> > /* PCI config space */
>
> Hm my docs say different. Supposedly the size field starts at bit 3
> and is 5 bits wide, going up to 512M. So just like SNB but with a
> bigger max.
>
So we discussed this a bit on IRC. I think we decided to leave it as is?
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 07/10] drm/i915: Calculate correct stolen size for GEN7+
2012-10-25 22:15 ` Ben Widawsky
@ 2012-10-25 22:39 ` Jesse Barnes
0 siblings, 0 replies; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 22:39 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, 25 Oct 2012 15:15:25 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> On Thu, 25 Oct 2012 14:06:48 -0700
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>
> > On Mon, 22 Oct 2012 18:34:12 -0700
> > Ben Widawsky <ben@bwidawsk.net> wrote:
> >
> > > This bug existed in the old code, but was easier to fix here in the
> > > rework. Unfortunately gen7 doesn't have a nice way to figure out the
> > > size and we must use a lookup table.
> > >
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > ---
> > > drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++++++-
> > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > > 2 files changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index 7d3ec42..16fe960 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -528,6 +528,15 @@ static inline unsigned int gen6_get_stolen_size(u16 snb_gmch_ctl)
> > > return snb_gmch_ctl << 25; /* 32 MB units */
> > > }
> > >
> > > +static inline unsigned int gen7_get_stolen_size(u16 snb_gmch_ctl)
> > > +{
> > > + static const int stolen_decoder[] = {
> > > + 0, 0, 0, 0, 0, 32, 48, 64, 128, 256, 96, 160, 224, 352};
> > > + snb_gmch_ctl >>= IVB_GMCH_GMS_SHIFT;
> > > + snb_gmch_ctl &= IVB_GMCH_GMS_MASK;
> > > + return stolen_decoder[snb_gmch_ctl] << 20;
> > > +}
> > > +
> > > int i915_gem_gtt_init(struct drm_device *dev)
> > > {
> > > struct drm_i915_private *dev_priv = dev->dev_private;
> > > @@ -575,7 +584,10 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > > pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
> > > dev_priv->mm.gtt->gtt_total_entries =
> > > gen6_get_total_gtt_size(snb_gmch_ctl) / sizeof(gtt_pte_t);
> > > - dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
> > > + if (INTEL_INFO(dev)->gen < 7)
> > > + dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
> > > + else
> > > + dev_priv->mm.gtt->stolen_size = gen7_get_stolen_size(snb_gmch_ctl);
> > >
> > > ret = setup_scratch_page(dev);
> > > if (ret)
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 10a6e9b..1d54328 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -45,6 +45,8 @@
> > > #define SNB_GMCH_GGMS_MASK 0x3
> > > #define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */
> > > #define SNB_GMCH_GMS_MASK 0x1f
> > > +#define IVB_GMCH_GMS_SHIFT 4
> > > +#define IVB_GMCH_GMS_MASK 0xf
> > >
> > >
> > > /* PCI config space */
> >
> > Hm my docs say different. Supposedly the size field starts at bit 3
> > and is 5 bits wide, going up to 512M. So just like SNB but with a
> > bigger max.
> >
>
> So we discussed this a bit on IRC. I think we decided to leave it as is?
Yeah, the docs don't agree, but we'll use the graphics ones and hope
for the best.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 08/10] drm/i915: Kill off now unused gen6+ AGP code
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
` (6 preceding siblings ...)
2012-10-23 1:34 ` [PATCH 07/10] drm/i915: Calculate correct stolen size for GEN7+ Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-25 21:07 ` Jesse Barnes
2012-10-23 1:34 ` [PATCH 09/10] drm/i915: flush system agent TLBs on SNB Ben Widawsky
` (2 subsequent siblings)
10 siblings, 1 reply; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/char/agp/intel-agp.h | 91 -------------
drivers/char/agp/intel-gtt.c | 307 +------------------------------------------
include/drm/intel-gtt.h | 4 -
3 files changed, 3 insertions(+), 399 deletions(-)
diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h
index 6ec0fff..1042c1b 100644
--- a/drivers/char/agp/intel-agp.h
+++ b/drivers/char/agp/intel-agp.h
@@ -62,12 +62,6 @@
#define I810_PTE_LOCAL 0x00000002
#define I810_PTE_VALID 0x00000001
#define I830_PTE_SYSTEM_CACHED 0x00000006
-/* GT PTE cache control fields */
-#define GEN6_PTE_UNCACHED 0x00000002
-#define HSW_PTE_UNCACHED 0x00000000
-#define GEN6_PTE_LLC 0x00000004
-#define GEN6_PTE_LLC_MLC 0x00000006
-#define GEN6_PTE_GFDT 0x00000008
#define I810_SMRAM_MISCC 0x70
#define I810_GFX_MEM_WIN_SIZE 0x00010000
@@ -97,7 +91,6 @@
#define G4x_GMCH_SIZE_VT_2M (G4x_GMCH_SIZE_2M | G4x_GMCH_SIZE_VT_EN)
#define GFX_FLSH_CNTL 0x2170 /* 915+ */
-#define GFX_FLSH_CNTL_VLV 0x101008
#define I810_DRAM_CTL 0x3000
#define I810_DRAM_ROW_0 0x00000001
@@ -148,29 +141,6 @@
#define INTEL_I7505_AGPCTRL 0x70
#define INTEL_I7505_MCHCFG 0x50
-#define SNB_GMCH_CTRL 0x50
-#define SNB_GMCH_GMS_STOLEN_MASK 0xF8
-#define SNB_GMCH_GMS_STOLEN_32M (1 << 3)
-#define SNB_GMCH_GMS_STOLEN_64M (2 << 3)
-#define SNB_GMCH_GMS_STOLEN_96M (3 << 3)
-#define SNB_GMCH_GMS_STOLEN_128M (4 << 3)
-#define SNB_GMCH_GMS_STOLEN_160M (5 << 3)
-#define SNB_GMCH_GMS_STOLEN_192M (6 << 3)
-#define SNB_GMCH_GMS_STOLEN_224M (7 << 3)
-#define SNB_GMCH_GMS_STOLEN_256M (8 << 3)
-#define SNB_GMCH_GMS_STOLEN_288M (9 << 3)
-#define SNB_GMCH_GMS_STOLEN_320M (0xa << 3)
-#define SNB_GMCH_GMS_STOLEN_352M (0xb << 3)
-#define SNB_GMCH_GMS_STOLEN_384M (0xc << 3)
-#define SNB_GMCH_GMS_STOLEN_416M (0xd << 3)
-#define SNB_GMCH_GMS_STOLEN_448M (0xe << 3)
-#define SNB_GMCH_GMS_STOLEN_480M (0xf << 3)
-#define SNB_GMCH_GMS_STOLEN_512M (0x10 << 3)
-#define SNB_GTT_SIZE_0M (0 << 8)
-#define SNB_GTT_SIZE_1M (1 << 8)
-#define SNB_GTT_SIZE_2M (2 << 8)
-#define SNB_GTT_SIZE_MASK (3 << 8)
-
/* pci devices ids */
#define PCI_DEVICE_ID_INTEL_E7221_HB 0x2588
#define PCI_DEVICE_ID_INTEL_E7221_IG 0x258a
@@ -219,66 +189,5 @@
#define PCI_DEVICE_ID_INTEL_IRONLAKE_MA_HB 0x0062
#define PCI_DEVICE_ID_INTEL_IRONLAKE_MC2_HB 0x006a
#define PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG 0x0046
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_HB 0x0100 /* Desktop */
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT1_IG 0x0102
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT2_IG 0x0112
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT2_PLUS_IG 0x0122
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_HB 0x0104 /* Mobile */
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT1_IG 0x0106
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT2_IG 0x0116
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT2_PLUS_IG 0x0126
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_S_HB 0x0108 /* Server */
-#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_S_IG 0x010A
-#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_HB 0x0150 /* Desktop */
-#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_GT1_IG 0x0152
-#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_GT2_IG 0x0162
-#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_HB 0x0154 /* Mobile */
-#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_GT1_IG 0x0156
-#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_GT2_IG 0x0166
-#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_HB 0x0158 /* Server */
-#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_GT1_IG 0x015A
-#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_GT2_IG 0x016A
-#define PCI_DEVICE_ID_INTEL_VALLEYVIEW_HB 0x0F00 /* VLV1 */
-#define PCI_DEVICE_ID_INTEL_VALLEYVIEW_IG 0x0F30
-#define PCI_DEVICE_ID_INTEL_HASWELL_HB 0x0400 /* Desktop */
-#define PCI_DEVICE_ID_INTEL_HASWELL_D_GT1_IG 0x0402
-#define PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_IG 0x0412
-#define PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_PLUS_IG 0x0422
-#define PCI_DEVICE_ID_INTEL_HASWELL_M_HB 0x0404 /* Mobile */
-#define PCI_DEVICE_ID_INTEL_HASWELL_M_GT1_IG 0x0406
-#define PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_IG 0x0416
-#define PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_PLUS_IG 0x0426
-#define PCI_DEVICE_ID_INTEL_HASWELL_S_HB 0x0408 /* Server */
-#define PCI_DEVICE_ID_INTEL_HASWELL_S_GT1_IG 0x040a
-#define PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_IG 0x041a
-#define PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_PLUS_IG 0x042a
-#define PCI_DEVICE_ID_INTEL_HASWELL_E_HB 0x0c04
-#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT1_IG 0x0C02
-#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_IG 0x0C12
-#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_PLUS_IG 0x0C22
-#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT1_IG 0x0C06
-#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT2_IG 0x0C16
-#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT2_PLUS_IG 0x0C26
-#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT1_IG 0x0C0A
-#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT2_IG 0x0C1A
-#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT2_PLUS_IG 0x0C2A
-#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT1_IG 0x0A02
-#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT2_IG 0x0A12
-#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT2_PLUS_IG 0x0A22
-#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT1_IG 0x0A06
-#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT2_IG 0x0A16
-#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT2_PLUS_IG 0x0A26
-#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT1_IG 0x0A0A
-#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT2_IG 0x0A1A
-#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT2_PLUS_IG 0x0A2A
-#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT1_IG 0x0D12
-#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT2_IG 0x0D22
-#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT2_PLUS_IG 0x0D32
-#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT1_IG 0x0D16
-#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT2_IG 0x0D26
-#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT2_PLUS_IG 0x0D36
-#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT1_IG 0x0D1A
-#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT2_IG 0x0D2A
-#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT2_PLUS_IG 0x0D3A
#endif
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 4dfbb80..c2d6002 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -367,62 +367,6 @@ static unsigned int intel_gtt_stolen_size(void)
stolen_size = 0;
break;
}
- } else if (INTEL_GTT_GEN == 6) {
- /*
- * SandyBridge has new memory control reg at 0x50.w
- */
- u16 snb_gmch_ctl;
- pci_read_config_word(intel_private.pcidev, SNB_GMCH_CTRL, &snb_gmch_ctl);
- switch (snb_gmch_ctl & SNB_GMCH_GMS_STOLEN_MASK) {
- case SNB_GMCH_GMS_STOLEN_32M:
- stolen_size = MB(32);
- break;
- case SNB_GMCH_GMS_STOLEN_64M:
- stolen_size = MB(64);
- break;
- case SNB_GMCH_GMS_STOLEN_96M:
- stolen_size = MB(96);
- break;
- case SNB_GMCH_GMS_STOLEN_128M:
- stolen_size = MB(128);
- break;
- case SNB_GMCH_GMS_STOLEN_160M:
- stolen_size = MB(160);
- break;
- case SNB_GMCH_GMS_STOLEN_192M:
- stolen_size = MB(192);
- break;
- case SNB_GMCH_GMS_STOLEN_224M:
- stolen_size = MB(224);
- break;
- case SNB_GMCH_GMS_STOLEN_256M:
- stolen_size = MB(256);
- break;
- case SNB_GMCH_GMS_STOLEN_288M:
- stolen_size = MB(288);
- break;
- case SNB_GMCH_GMS_STOLEN_320M:
- stolen_size = MB(320);
- break;
- case SNB_GMCH_GMS_STOLEN_352M:
- stolen_size = MB(352);
- break;
- case SNB_GMCH_GMS_STOLEN_384M:
- stolen_size = MB(384);
- break;
- case SNB_GMCH_GMS_STOLEN_416M:
- stolen_size = MB(416);
- break;
- case SNB_GMCH_GMS_STOLEN_448M:
- stolen_size = MB(448);
- break;
- case SNB_GMCH_GMS_STOLEN_480M:
- stolen_size = MB(480);
- break;
- case SNB_GMCH_GMS_STOLEN_512M:
- stolen_size = MB(512);
- break;
- }
} else {
switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
case I855_GMCH_GMS_STOLEN_1M:
@@ -556,29 +500,9 @@ static unsigned int i965_gtt_total_entries(void)
static unsigned int intel_gtt_total_entries(void)
{
- int size;
-
if (IS_G33 || INTEL_GTT_GEN == 4 || INTEL_GTT_GEN == 5)
return i965_gtt_total_entries();
- else if (INTEL_GTT_GEN == 6) {
- u16 snb_gmch_ctl;
-
- pci_read_config_word(intel_private.pcidev, SNB_GMCH_CTRL, &snb_gmch_ctl);
- switch (snb_gmch_ctl & SNB_GTT_SIZE_MASK) {
- default:
- case SNB_GTT_SIZE_0M:
- printk(KERN_ERR "Bad GTT size mask: 0x%04x.\n", snb_gmch_ctl);
- size = MB(0);
- break;
- case SNB_GTT_SIZE_1M:
- size = MB(1);
- break;
- case SNB_GTT_SIZE_2M:
- size = MB(2);
- break;
- }
- return size/4;
- } else {
+ else {
/* On previous hardware, the GTT size was just what was
* required to map the aperture.
*/
@@ -778,9 +702,6 @@ bool intel_enable_gtt(void)
{
u8 __iomem *reg;
- if (INTEL_GTT_GEN >= 6)
- return true;
-
if (INTEL_GTT_GEN == 2) {
u16 gmch_ctrl;
@@ -1149,85 +1070,6 @@ static void i965_write_entry(dma_addr_t addr,
writel(addr | pte_flags, intel_private.gtt + entry);
}
-static bool gen6_check_flags(unsigned int flags)
-{
- return true;
-}
-
-static void haswell_write_entry(dma_addr_t addr, unsigned int entry,
- unsigned int flags)
-{
- unsigned int type_mask = flags & ~AGP_USER_CACHED_MEMORY_GFDT;
- unsigned int gfdt = flags & AGP_USER_CACHED_MEMORY_GFDT;
- u32 pte_flags;
-
- if (type_mask == AGP_USER_MEMORY)
- pte_flags = HSW_PTE_UNCACHED | I810_PTE_VALID;
- else if (type_mask == AGP_USER_CACHED_MEMORY_LLC_MLC) {
- pte_flags = GEN6_PTE_LLC_MLC | I810_PTE_VALID;
- if (gfdt)
- pte_flags |= GEN6_PTE_GFDT;
- } else { /* set 'normal'/'cached' to LLC by default */
- pte_flags = GEN6_PTE_LLC | I810_PTE_VALID;
- if (gfdt)
- pte_flags |= GEN6_PTE_GFDT;
- }
-
- /* gen6 has bit11-4 for physical addr bit39-32 */
- addr |= (addr >> 28) & 0xff0;
- writel(addr | pte_flags, intel_private.gtt + entry);
-}
-
-static void gen6_write_entry(dma_addr_t addr, unsigned int entry,
- unsigned int flags)
-{
- unsigned int type_mask = flags & ~AGP_USER_CACHED_MEMORY_GFDT;
- unsigned int gfdt = flags & AGP_USER_CACHED_MEMORY_GFDT;
- u32 pte_flags;
-
- if (type_mask == AGP_USER_MEMORY)
- pte_flags = GEN6_PTE_UNCACHED | I810_PTE_VALID;
- else if (type_mask == AGP_USER_CACHED_MEMORY_LLC_MLC) {
- pte_flags = GEN6_PTE_LLC_MLC | I810_PTE_VALID;
- if (gfdt)
- pte_flags |= GEN6_PTE_GFDT;
- } else { /* set 'normal'/'cached' to LLC by default */
- pte_flags = GEN6_PTE_LLC | I810_PTE_VALID;
- if (gfdt)
- pte_flags |= GEN6_PTE_GFDT;
- }
-
- /* gen6 has bit11-4 for physical addr bit39-32 */
- addr |= (addr >> 28) & 0xff0;
- writel(addr | pte_flags, intel_private.gtt + entry);
-}
-
-static void valleyview_write_entry(dma_addr_t addr, unsigned int entry,
- unsigned int flags)
-{
- unsigned int type_mask = flags & ~AGP_USER_CACHED_MEMORY_GFDT;
- unsigned int gfdt = flags & AGP_USER_CACHED_MEMORY_GFDT;
- u32 pte_flags;
-
- if (type_mask == AGP_USER_MEMORY)
- pte_flags = GEN6_PTE_UNCACHED | I810_PTE_VALID;
- else {
- pte_flags = GEN6_PTE_LLC | I810_PTE_VALID;
- if (gfdt)
- pte_flags |= GEN6_PTE_GFDT;
- }
-
- /* gen6 has bit11-4 for physical addr bit39-32 */
- addr |= (addr >> 28) & 0xff0;
- writel(addr | pte_flags, intel_private.gtt + entry);
-
- writel(1, intel_private.registers + GFX_FLSH_CNTL_VLV);
-}
-
-static void gen6_cleanup(void)
-{
-}
-
/* Certain Gen5 chipsets require require idling the GPU before
* unmapping anything from the GTT when VT-d is enabled.
*/
@@ -1256,9 +1098,6 @@ static int i9xx_setup(void)
reg_addr &= 0xfff80000;
- if (INTEL_GTT_GEN >= 7)
- size = MB(2);
-
intel_private.registers = ioremap(reg_addr, size);
if (!intel_private.registers)
return -ENOMEM;
@@ -1269,22 +1108,8 @@ static int i9xx_setup(void)
pci_read_config_dword(intel_private.pcidev,
I915_PTEADDR, >t_addr);
intel_private.gtt_bus_addr = gtt_addr;
- } else {
- u32 gtt_offset;
-
- switch (INTEL_GTT_GEN) {
- case 5:
- case 6:
- case 7:
- gtt_offset = MB(2);
- break;
- case 4:
- default:
- gtt_offset = KB(512);
- break;
- }
- intel_private.gtt_bus_addr = reg_addr + gtt_offset;
- }
+ } else
+ intel_private.gtt_bus_addr = reg_addr + KB(512);
if (needs_idle_maps())
intel_private.base.do_idle_maps = 1;
@@ -1395,32 +1220,6 @@ static const struct intel_gtt_driver ironlake_gtt_driver = {
.check_flags = i830_check_flags,
.chipset_flush = i9xx_chipset_flush,
};
-static const struct intel_gtt_driver sandybridge_gtt_driver = {
- .gen = 6,
- .setup = i9xx_setup,
- .cleanup = gen6_cleanup,
- .write_entry = gen6_write_entry,
- .dma_mask_size = 40,
- .check_flags = gen6_check_flags,
- .chipset_flush = i9xx_chipset_flush,
-};
-static const struct intel_gtt_driver haswell_gtt_driver = {
- .gen = 6,
- .setup = i9xx_setup,
- .cleanup = gen6_cleanup,
- .write_entry = haswell_write_entry,
- .dma_mask_size = 40,
- .check_flags = gen6_check_flags,
- .chipset_flush = i9xx_chipset_flush,
-};
-static const struct intel_gtt_driver valleyview_gtt_driver = {
- .gen = 7,
- .setup = i9xx_setup,
- .cleanup = gen6_cleanup,
- .write_entry = valleyview_write_entry,
- .dma_mask_size = 40,
- .check_flags = gen6_check_flags,
-};
/* Table to describe Intel GMCH and AGP/PCIE GART drivers. At least one of
* driver and gmch_driver must be non-null, and find_gmch will determine
@@ -1501,106 +1300,6 @@ static const struct intel_gtt_driver_description {
"HD Graphics", &ironlake_gtt_driver },
{ PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG,
"HD Graphics", &ironlake_gtt_driver },
- { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT1_IG,
- "Sandybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT2_IG,
- "Sandybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT2_PLUS_IG,
- "Sandybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT1_IG,
- "Sandybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT2_IG,
- "Sandybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT2_PLUS_IG,
- "Sandybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_S_IG,
- "Sandybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_IVYBRIDGE_GT1_IG,
- "Ivybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_IVYBRIDGE_GT2_IG,
- "Ivybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_GT1_IG,
- "Ivybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_GT2_IG,
- "Ivybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_GT1_IG,
- "Ivybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_GT2_IG,
- "Ivybridge", &sandybridge_gtt_driver },
- { PCI_DEVICE_ID_INTEL_VALLEYVIEW_IG,
- "ValleyView", &valleyview_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_D_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_M_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_S_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT1_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT2_IG,
- "Haswell", &haswell_gtt_driver },
- { PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT2_PLUS_IG,
- "Haswell", &haswell_gtt_driver },
{ 0, NULL, NULL }
};
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 94e8f2c..6eb76a1 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -40,10 +40,6 @@ void intel_gtt_clear_range(unsigned int first_entry, unsigned int num_entries);
#define AGP_DCACHE_MEMORY 1
#define AGP_PHYS_MEMORY 2
-/* New caching attributes for gen6/sandybridge */
-#define AGP_USER_CACHED_MEMORY_LLC_MLC (AGP_USER_TYPES + 2)
-#define AGP_USER_UNCACHED_MEMORY (AGP_USER_TYPES + 4)
-
/* flag for GFDT type */
#define AGP_USER_CACHED_MEMORY_GFDT (1 << 3)
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 08/10] drm/i915: Kill off now unused gen6+ AGP code
2012-10-23 1:34 ` [PATCH 08/10] drm/i915: Kill off now unused gen6+ AGP code Ben Widawsky
@ 2012-10-25 21:07 ` Jesse Barnes
0 siblings, 0 replies; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 21:07 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:13 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/char/agp/intel-agp.h | 91 -------------
> drivers/char/agp/intel-gtt.c | 307 +------------------------------------------
> include/drm/intel-gtt.h | 4 -
> 3 files changed, 3 insertions(+), 399 deletions(-)
>
> diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h
> index 6ec0fff..1042c1b 100644
> --- a/drivers/char/agp/intel-agp.h
> +++ b/drivers/char/agp/intel-agp.h
> @@ -62,12 +62,6 @@
> #define I810_PTE_LOCAL 0x00000002
> #define I810_PTE_VALID 0x00000001
> #define I830_PTE_SYSTEM_CACHED 0x00000006
> -/* GT PTE cache control fields */
> -#define GEN6_PTE_UNCACHED 0x00000002
> -#define HSW_PTE_UNCACHED 0x00000000
> -#define GEN6_PTE_LLC 0x00000004
> -#define GEN6_PTE_LLC_MLC 0x00000006
> -#define GEN6_PTE_GFDT 0x00000008
>
> #define I810_SMRAM_MISCC 0x70
> #define I810_GFX_MEM_WIN_SIZE 0x00010000
> @@ -97,7 +91,6 @@
> #define G4x_GMCH_SIZE_VT_2M (G4x_GMCH_SIZE_2M | G4x_GMCH_SIZE_VT_EN)
>
> #define GFX_FLSH_CNTL 0x2170 /* 915+ */
> -#define GFX_FLSH_CNTL_VLV 0x101008
>
> #define I810_DRAM_CTL 0x3000
> #define I810_DRAM_ROW_0 0x00000001
> @@ -148,29 +141,6 @@
> #define INTEL_I7505_AGPCTRL 0x70
> #define INTEL_I7505_MCHCFG 0x50
>
> -#define SNB_GMCH_CTRL 0x50
> -#define SNB_GMCH_GMS_STOLEN_MASK 0xF8
> -#define SNB_GMCH_GMS_STOLEN_32M (1 << 3)
> -#define SNB_GMCH_GMS_STOLEN_64M (2 << 3)
> -#define SNB_GMCH_GMS_STOLEN_96M (3 << 3)
> -#define SNB_GMCH_GMS_STOLEN_128M (4 << 3)
> -#define SNB_GMCH_GMS_STOLEN_160M (5 << 3)
> -#define SNB_GMCH_GMS_STOLEN_192M (6 << 3)
> -#define SNB_GMCH_GMS_STOLEN_224M (7 << 3)
> -#define SNB_GMCH_GMS_STOLEN_256M (8 << 3)
> -#define SNB_GMCH_GMS_STOLEN_288M (9 << 3)
> -#define SNB_GMCH_GMS_STOLEN_320M (0xa << 3)
> -#define SNB_GMCH_GMS_STOLEN_352M (0xb << 3)
> -#define SNB_GMCH_GMS_STOLEN_384M (0xc << 3)
> -#define SNB_GMCH_GMS_STOLEN_416M (0xd << 3)
> -#define SNB_GMCH_GMS_STOLEN_448M (0xe << 3)
> -#define SNB_GMCH_GMS_STOLEN_480M (0xf << 3)
> -#define SNB_GMCH_GMS_STOLEN_512M (0x10 << 3)
> -#define SNB_GTT_SIZE_0M (0 << 8)
> -#define SNB_GTT_SIZE_1M (1 << 8)
> -#define SNB_GTT_SIZE_2M (2 << 8)
> -#define SNB_GTT_SIZE_MASK (3 << 8)
> -
> /* pci devices ids */
> #define PCI_DEVICE_ID_INTEL_E7221_HB 0x2588
> #define PCI_DEVICE_ID_INTEL_E7221_IG 0x258a
> @@ -219,66 +189,5 @@
> #define PCI_DEVICE_ID_INTEL_IRONLAKE_MA_HB 0x0062
> #define PCI_DEVICE_ID_INTEL_IRONLAKE_MC2_HB 0x006a
> #define PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG 0x0046
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_HB 0x0100 /* Desktop */
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT1_IG 0x0102
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT2_IG 0x0112
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT2_PLUS_IG 0x0122
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_HB 0x0104 /* Mobile */
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT1_IG 0x0106
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT2_IG 0x0116
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT2_PLUS_IG 0x0126
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_S_HB 0x0108 /* Server */
> -#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_S_IG 0x010A
> -#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_HB 0x0150 /* Desktop */
> -#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_GT1_IG 0x0152
> -#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_GT2_IG 0x0162
> -#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_HB 0x0154 /* Mobile */
> -#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_GT1_IG 0x0156
> -#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_GT2_IG 0x0166
> -#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_HB 0x0158 /* Server */
> -#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_GT1_IG 0x015A
> -#define PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_GT2_IG 0x016A
> -#define PCI_DEVICE_ID_INTEL_VALLEYVIEW_HB 0x0F00 /* VLV1 */
> -#define PCI_DEVICE_ID_INTEL_VALLEYVIEW_IG 0x0F30
> -#define PCI_DEVICE_ID_INTEL_HASWELL_HB 0x0400 /* Desktop */
> -#define PCI_DEVICE_ID_INTEL_HASWELL_D_GT1_IG 0x0402
> -#define PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_IG 0x0412
> -#define PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_PLUS_IG 0x0422
> -#define PCI_DEVICE_ID_INTEL_HASWELL_M_HB 0x0404 /* Mobile */
> -#define PCI_DEVICE_ID_INTEL_HASWELL_M_GT1_IG 0x0406
> -#define PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_IG 0x0416
> -#define PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_PLUS_IG 0x0426
> -#define PCI_DEVICE_ID_INTEL_HASWELL_S_HB 0x0408 /* Server */
> -#define PCI_DEVICE_ID_INTEL_HASWELL_S_GT1_IG 0x040a
> -#define PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_IG 0x041a
> -#define PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_PLUS_IG 0x042a
> -#define PCI_DEVICE_ID_INTEL_HASWELL_E_HB 0x0c04
> -#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT1_IG 0x0C02
> -#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_IG 0x0C12
> -#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_PLUS_IG 0x0C22
> -#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT1_IG 0x0C06
> -#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT2_IG 0x0C16
> -#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT2_PLUS_IG 0x0C26
> -#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT1_IG 0x0C0A
> -#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT2_IG 0x0C1A
> -#define PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT2_PLUS_IG 0x0C2A
> -#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT1_IG 0x0A02
> -#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT2_IG 0x0A12
> -#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT2_PLUS_IG 0x0A22
> -#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT1_IG 0x0A06
> -#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT2_IG 0x0A16
> -#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT2_PLUS_IG 0x0A26
> -#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT1_IG 0x0A0A
> -#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT2_IG 0x0A1A
> -#define PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT2_PLUS_IG 0x0A2A
> -#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT1_IG 0x0D12
> -#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT2_IG 0x0D22
> -#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT2_PLUS_IG 0x0D32
> -#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT1_IG 0x0D16
> -#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT2_IG 0x0D26
> -#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT2_PLUS_IG 0x0D36
> -#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT1_IG 0x0D1A
> -#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT2_IG 0x0D2A
> -#define PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT2_PLUS_IG 0x0D3A
>
> #endif
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index 4dfbb80..c2d6002 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -367,62 +367,6 @@ static unsigned int intel_gtt_stolen_size(void)
> stolen_size = 0;
> break;
> }
> - } else if (INTEL_GTT_GEN == 6) {
> - /*
> - * SandyBridge has new memory control reg at 0x50.w
> - */
> - u16 snb_gmch_ctl;
> - pci_read_config_word(intel_private.pcidev, SNB_GMCH_CTRL, &snb_gmch_ctl);
> - switch (snb_gmch_ctl & SNB_GMCH_GMS_STOLEN_MASK) {
> - case SNB_GMCH_GMS_STOLEN_32M:
> - stolen_size = MB(32);
> - break;
> - case SNB_GMCH_GMS_STOLEN_64M:
> - stolen_size = MB(64);
> - break;
> - case SNB_GMCH_GMS_STOLEN_96M:
> - stolen_size = MB(96);
> - break;
> - case SNB_GMCH_GMS_STOLEN_128M:
> - stolen_size = MB(128);
> - break;
> - case SNB_GMCH_GMS_STOLEN_160M:
> - stolen_size = MB(160);
> - break;
> - case SNB_GMCH_GMS_STOLEN_192M:
> - stolen_size = MB(192);
> - break;
> - case SNB_GMCH_GMS_STOLEN_224M:
> - stolen_size = MB(224);
> - break;
> - case SNB_GMCH_GMS_STOLEN_256M:
> - stolen_size = MB(256);
> - break;
> - case SNB_GMCH_GMS_STOLEN_288M:
> - stolen_size = MB(288);
> - break;
> - case SNB_GMCH_GMS_STOLEN_320M:
> - stolen_size = MB(320);
> - break;
> - case SNB_GMCH_GMS_STOLEN_352M:
> - stolen_size = MB(352);
> - break;
> - case SNB_GMCH_GMS_STOLEN_384M:
> - stolen_size = MB(384);
> - break;
> - case SNB_GMCH_GMS_STOLEN_416M:
> - stolen_size = MB(416);
> - break;
> - case SNB_GMCH_GMS_STOLEN_448M:
> - stolen_size = MB(448);
> - break;
> - case SNB_GMCH_GMS_STOLEN_480M:
> - stolen_size = MB(480);
> - break;
> - case SNB_GMCH_GMS_STOLEN_512M:
> - stolen_size = MB(512);
> - break;
> - }
> } else {
> switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> case I855_GMCH_GMS_STOLEN_1M:
> @@ -556,29 +500,9 @@ static unsigned int i965_gtt_total_entries(void)
>
> static unsigned int intel_gtt_total_entries(void)
> {
> - int size;
> -
> if (IS_G33 || INTEL_GTT_GEN == 4 || INTEL_GTT_GEN == 5)
> return i965_gtt_total_entries();
> - else if (INTEL_GTT_GEN == 6) {
> - u16 snb_gmch_ctl;
> -
> - pci_read_config_word(intel_private.pcidev, SNB_GMCH_CTRL, &snb_gmch_ctl);
> - switch (snb_gmch_ctl & SNB_GTT_SIZE_MASK) {
> - default:
> - case SNB_GTT_SIZE_0M:
> - printk(KERN_ERR "Bad GTT size mask: 0x%04x.\n", snb_gmch_ctl);
> - size = MB(0);
> - break;
> - case SNB_GTT_SIZE_1M:
> - size = MB(1);
> - break;
> - case SNB_GTT_SIZE_2M:
> - size = MB(2);
> - break;
> - }
> - return size/4;
> - } else {
> + else {
> /* On previous hardware, the GTT size was just what was
> * required to map the aperture.
> */
> @@ -778,9 +702,6 @@ bool intel_enable_gtt(void)
> {
> u8 __iomem *reg;
>
> - if (INTEL_GTT_GEN >= 6)
> - return true;
> -
> if (INTEL_GTT_GEN == 2) {
> u16 gmch_ctrl;
>
> @@ -1149,85 +1070,6 @@ static void i965_write_entry(dma_addr_t addr,
> writel(addr | pte_flags, intel_private.gtt + entry);
> }
>
> -static bool gen6_check_flags(unsigned int flags)
> -{
> - return true;
> -}
> -
> -static void haswell_write_entry(dma_addr_t addr, unsigned int entry,
> - unsigned int flags)
> -{
> - unsigned int type_mask = flags & ~AGP_USER_CACHED_MEMORY_GFDT;
> - unsigned int gfdt = flags & AGP_USER_CACHED_MEMORY_GFDT;
> - u32 pte_flags;
> -
> - if (type_mask == AGP_USER_MEMORY)
> - pte_flags = HSW_PTE_UNCACHED | I810_PTE_VALID;
> - else if (type_mask == AGP_USER_CACHED_MEMORY_LLC_MLC) {
> - pte_flags = GEN6_PTE_LLC_MLC | I810_PTE_VALID;
> - if (gfdt)
> - pte_flags |= GEN6_PTE_GFDT;
> - } else { /* set 'normal'/'cached' to LLC by default */
> - pte_flags = GEN6_PTE_LLC | I810_PTE_VALID;
> - if (gfdt)
> - pte_flags |= GEN6_PTE_GFDT;
> - }
> -
> - /* gen6 has bit11-4 for physical addr bit39-32 */
> - addr |= (addr >> 28) & 0xff0;
> - writel(addr | pte_flags, intel_private.gtt + entry);
> -}
> -
> -static void gen6_write_entry(dma_addr_t addr, unsigned int entry,
> - unsigned int flags)
> -{
> - unsigned int type_mask = flags & ~AGP_USER_CACHED_MEMORY_GFDT;
> - unsigned int gfdt = flags & AGP_USER_CACHED_MEMORY_GFDT;
> - u32 pte_flags;
> -
> - if (type_mask == AGP_USER_MEMORY)
> - pte_flags = GEN6_PTE_UNCACHED | I810_PTE_VALID;
> - else if (type_mask == AGP_USER_CACHED_MEMORY_LLC_MLC) {
> - pte_flags = GEN6_PTE_LLC_MLC | I810_PTE_VALID;
> - if (gfdt)
> - pte_flags |= GEN6_PTE_GFDT;
> - } else { /* set 'normal'/'cached' to LLC by default */
> - pte_flags = GEN6_PTE_LLC | I810_PTE_VALID;
> - if (gfdt)
> - pte_flags |= GEN6_PTE_GFDT;
> - }
> -
> - /* gen6 has bit11-4 for physical addr bit39-32 */
> - addr |= (addr >> 28) & 0xff0;
> - writel(addr | pte_flags, intel_private.gtt + entry);
> -}
> -
> -static void valleyview_write_entry(dma_addr_t addr, unsigned int entry,
> - unsigned int flags)
> -{
> - unsigned int type_mask = flags & ~AGP_USER_CACHED_MEMORY_GFDT;
> - unsigned int gfdt = flags & AGP_USER_CACHED_MEMORY_GFDT;
> - u32 pte_flags;
> -
> - if (type_mask == AGP_USER_MEMORY)
> - pte_flags = GEN6_PTE_UNCACHED | I810_PTE_VALID;
> - else {
> - pte_flags = GEN6_PTE_LLC | I810_PTE_VALID;
> - if (gfdt)
> - pte_flags |= GEN6_PTE_GFDT;
> - }
> -
> - /* gen6 has bit11-4 for physical addr bit39-32 */
> - addr |= (addr >> 28) & 0xff0;
> - writel(addr | pte_flags, intel_private.gtt + entry);
> -
> - writel(1, intel_private.registers + GFX_FLSH_CNTL_VLV);
> -}
> -
> -static void gen6_cleanup(void)
> -{
> -}
> -
> /* Certain Gen5 chipsets require require idling the GPU before
> * unmapping anything from the GTT when VT-d is enabled.
> */
> @@ -1256,9 +1098,6 @@ static int i9xx_setup(void)
>
> reg_addr &= 0xfff80000;
>
> - if (INTEL_GTT_GEN >= 7)
> - size = MB(2);
> -
> intel_private.registers = ioremap(reg_addr, size);
> if (!intel_private.registers)
> return -ENOMEM;
> @@ -1269,22 +1108,8 @@ static int i9xx_setup(void)
> pci_read_config_dword(intel_private.pcidev,
> I915_PTEADDR, >t_addr);
> intel_private.gtt_bus_addr = gtt_addr;
> - } else {
> - u32 gtt_offset;
> -
> - switch (INTEL_GTT_GEN) {
> - case 5:
> - case 6:
> - case 7:
> - gtt_offset = MB(2);
> - break;
> - case 4:
> - default:
> - gtt_offset = KB(512);
> - break;
> - }
> - intel_private.gtt_bus_addr = reg_addr + gtt_offset;
> - }
> + } else
> + intel_private.gtt_bus_addr = reg_addr + KB(512);
>
> if (needs_idle_maps())
> intel_private.base.do_idle_maps = 1;
> @@ -1395,32 +1220,6 @@ static const struct intel_gtt_driver ironlake_gtt_driver = {
> .check_flags = i830_check_flags,
> .chipset_flush = i9xx_chipset_flush,
> };
> -static const struct intel_gtt_driver sandybridge_gtt_driver = {
> - .gen = 6,
> - .setup = i9xx_setup,
> - .cleanup = gen6_cleanup,
> - .write_entry = gen6_write_entry,
> - .dma_mask_size = 40,
> - .check_flags = gen6_check_flags,
> - .chipset_flush = i9xx_chipset_flush,
> -};
> -static const struct intel_gtt_driver haswell_gtt_driver = {
> - .gen = 6,
> - .setup = i9xx_setup,
> - .cleanup = gen6_cleanup,
> - .write_entry = haswell_write_entry,
> - .dma_mask_size = 40,
> - .check_flags = gen6_check_flags,
> - .chipset_flush = i9xx_chipset_flush,
> -};
> -static const struct intel_gtt_driver valleyview_gtt_driver = {
> - .gen = 7,
> - .setup = i9xx_setup,
> - .cleanup = gen6_cleanup,
> - .write_entry = valleyview_write_entry,
> - .dma_mask_size = 40,
> - .check_flags = gen6_check_flags,
> -};
>
> /* Table to describe Intel GMCH and AGP/PCIE GART drivers. At least one of
> * driver and gmch_driver must be non-null, and find_gmch will determine
> @@ -1501,106 +1300,6 @@ static const struct intel_gtt_driver_description {
> "HD Graphics", &ironlake_gtt_driver },
> { PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG,
> "HD Graphics", &ironlake_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT1_IG,
> - "Sandybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT2_IG,
> - "Sandybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_GT2_PLUS_IG,
> - "Sandybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT1_IG,
> - "Sandybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT2_IG,
> - "Sandybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_GT2_PLUS_IG,
> - "Sandybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_SANDYBRIDGE_S_IG,
> - "Sandybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_IVYBRIDGE_GT1_IG,
> - "Ivybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_IVYBRIDGE_GT2_IG,
> - "Ivybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_GT1_IG,
> - "Ivybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_GT2_IG,
> - "Ivybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_GT1_IG,
> - "Ivybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_GT2_IG,
> - "Ivybridge", &sandybridge_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_VALLEYVIEW_IG,
> - "ValleyView", &valleyview_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_D_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_M_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_S_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_SDV_S_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_ULT_D_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_ULT_M_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_ULT_S_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_CRW_D_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_CRW_M_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT1_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT2_IG,
> - "Haswell", &haswell_gtt_driver },
> - { PCI_DEVICE_ID_INTEL_HASWELL_CRW_S_GT2_PLUS_IG,
> - "Haswell", &haswell_gtt_driver },
> { 0, NULL, NULL }
> };
>
> diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
> index 94e8f2c..6eb76a1 100644
> --- a/include/drm/intel-gtt.h
> +++ b/include/drm/intel-gtt.h
> @@ -40,10 +40,6 @@ void intel_gtt_clear_range(unsigned int first_entry, unsigned int num_entries);
> #define AGP_DCACHE_MEMORY 1
> #define AGP_PHYS_MEMORY 2
>
> -/* New caching attributes for gen6/sandybridge */
> -#define AGP_USER_CACHED_MEMORY_LLC_MLC (AGP_USER_TYPES + 2)
> -#define AGP_USER_UNCACHED_MEMORY (AGP_USER_TYPES + 4)
> -
> /* flag for GFDT type */
> #define AGP_USER_CACHED_MEMORY_GFDT (1 << 3)
>
Yay.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 09/10] drm/i915: flush system agent TLBs on SNB
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
` (7 preceding siblings ...)
2012-10-23 1:34 ` [PATCH 08/10] drm/i915: Kill off now unused gen6+ AGP code Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-23 14:17 ` Jesse Barnes
` (2 more replies)
2012-10-23 1:34 ` [PATCH 10/10] drm/i915: Kill off actually requiring AGP Ben Widawsky
2012-10-26 20:58 ` [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
10 siblings, 3 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
This allows us to map the PTEs WC. I've not done thorough testing or
performance measurements with this patch, but it should be decent.
This is based on a patch from Jesse with the original commit message
> I've only lightly tested this so far, but the corruption seems to be
> gone if I write the GFX_FLSH_CNTL reg after binding an object. This
> register should control the TLB for the system agent, which is what CPU
> mapped objects will go through.
It has been updated for the new AGP-less code by me, and included with
it is feedback from the original patch.
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
drivers/gpu/drm/i915/i915_reg.h | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 16fe960..e5f0a7f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -391,6 +391,7 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
}
out:
+ I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
/* XXX: This serves as a posting read preserving the way the old code
* works. It's not clear if this is strictly necessary or just voodoo
* based on what I've tried to gather from the docs.
@@ -593,8 +594,8 @@ int i915_gem_gtt_init(struct drm_device *dev)
if (ret)
return ret;
- dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
- dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
+ dev_priv->mm.gtt->gtt = ioremap_wc(gtt_bus_addr,
+ dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
/* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1d54328..e8c578f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -683,6 +683,8 @@
#define CM0_RC_OP_FLUSH_DISABLE (1<<0)
#define BB_ADDR 0x02140 /* 8 bytes */
#define GFX_FLSH_CNTL 0x02170 /* 915+ only */
+#define GFX_FLSH_CNTL_GEN6 0x101008
+#define GFX_FLSH_CNTL_EN (1<<0)
#define ECOSKPD 0x021d0
#define ECO_GATING_CX_ONLY (1<<3)
#define ECO_FLIP_DONE (1<<0)
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 09/10] drm/i915: flush system agent TLBs on SNB
2012-10-23 1:34 ` [PATCH 09/10] drm/i915: flush system agent TLBs on SNB Ben Widawsky
@ 2012-10-23 14:17 ` Jesse Barnes
2012-10-23 14:28 ` Daniel Vetter
2012-10-25 21:07 ` Jesse Barnes
2012-10-26 20:55 ` [PATCH 09/10 v2] " Ben Widawsky
2 siblings, 1 reply; 50+ messages in thread
From: Jesse Barnes @ 2012-10-23 14:17 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:14 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> This allows us to map the PTEs WC. I've not done thorough testing or
> performance measurements with this patch, but it should be decent.
>
> This is based on a patch from Jesse with the original commit message
> > I've only lightly tested this so far, but the corruption seems to be
> > gone if I write the GFX_FLSH_CNTL reg after binding an object. This
> > register should control the TLB for the system agent, which is what CPU
> > mapped objects will go through.
>
> It has been updated for the new AGP-less code by me, and included with
> it is feedback from the original patch.
>
> CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 16fe960..e5f0a7f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -391,6 +391,7 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
> }
>
> out:
> + I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> /* XXX: This serves as a posting read preserving the way the old code
> * works. It's not clear if this is strictly necessary or just voodoo
> * based on what I've tried to gather from the docs.
> @@ -593,8 +594,8 @@ int i915_gem_gtt_init(struct drm_device *dev)
> if (ret)
> return ret;
>
> - dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
> - dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> + dev_priv->mm.gtt->gtt = ioremap_wc(gtt_bus_addr,
> + dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
>
> /* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
> DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1d54328..e8c578f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -683,6 +683,8 @@
> #define CM0_RC_OP_FLUSH_DISABLE (1<<0)
> #define BB_ADDR 0x02140 /* 8 bytes */
> #define GFX_FLSH_CNTL 0x02170 /* 915+ only */
> +#define GFX_FLSH_CNTL_GEN6 0x101008
> +#define GFX_FLSH_CNTL_EN (1<<0)
> #define ECOSKPD 0x021d0
> #define ECO_GATING_CX_ONLY (1<<3)
> #define ECO_FLIP_DONE (1<<0)
Looks good. Has anyone tried this on gen3-5?
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 09/10] drm/i915: flush system agent TLBs on SNB
2012-10-23 14:17 ` Jesse Barnes
@ 2012-10-23 14:28 ` Daniel Vetter
2012-10-23 14:35 ` Jesse Barnes
0 siblings, 1 reply; 50+ messages in thread
From: Daniel Vetter @ 2012-10-23 14:28 UTC (permalink / raw)
To: Jesse Barnes; +Cc: Ben Widawsky, intel-gfx
On Tue, Oct 23, 2012 at 07:17:53AM -0700, Jesse Barnes wrote:
> On Mon, 22 Oct 2012 18:34:14 -0700
> Ben Widawsky <ben@bwidawsk.net> wrote:
>
> > This allows us to map the PTEs WC. I've not done thorough testing or
> > performance measurements with this patch, but it should be decent.
> >
> > This is based on a patch from Jesse with the original commit message
> > > I've only lightly tested this so far, but the corruption seems to be
> > > gone if I write the GFX_FLSH_CNTL reg after binding an object. This
> > > register should control the TLB for the system agent, which is what CPU
> > > mapped objects will go through.
> >
> > It has been updated for the new AGP-less code by me, and included with
> > it is feedback from the original patch.
> >
> > CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > 2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 16fe960..e5f0a7f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -391,6 +391,7 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
> > }
> >
> > out:
> > + I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> > /* XXX: This serves as a posting read preserving the way the old code
> > * works. It's not clear if this is strictly necessary or just voodoo
> > * based on what I've tried to gather from the docs.
> > @@ -593,8 +594,8 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > if (ret)
> > return ret;
> >
> > - dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
> > - dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> > + dev_priv->mm.gtt->gtt = ioremap_wc(gtt_bus_addr,
> > + dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> >
> > /* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
> > DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 1d54328..e8c578f 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -683,6 +683,8 @@
> > #define CM0_RC_OP_FLUSH_DISABLE (1<<0)
> > #define BB_ADDR 0x02140 /* 8 bytes */
> > #define GFX_FLSH_CNTL 0x02170 /* 915+ only */
> > +#define GFX_FLSH_CNTL_GEN6 0x101008
> > +#define GFX_FLSH_CNTL_EN (1<<0)
> > #define ECOSKPD 0x021d0
> > #define ECO_GATING_CX_ONLY (1<<3)
> > #define ECO_FLIP_DONE (1<<0)
>
> Looks good. Has anyone tried this on gen3-5?
pre-gen6 still use the intel_gtt code, since we need to for the next
decade or so to keep ums (and crappy xvmc on gen3) going.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 09/10] drm/i915: flush system agent TLBs on SNB
2012-10-23 14:28 ` Daniel Vetter
@ 2012-10-23 14:35 ` Jesse Barnes
2012-10-23 14:42 ` Ben Widawsky
0 siblings, 1 reply; 50+ messages in thread
From: Jesse Barnes @ 2012-10-23 14:35 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Ben Widawsky, intel-gfx
On Tue, 23 Oct 2012 16:28:53 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Oct 23, 2012 at 07:17:53AM -0700, Jesse Barnes wrote:
> > On Mon, 22 Oct 2012 18:34:14 -0700
> > Ben Widawsky <ben@bwidawsk.net> wrote:
> >
> > > This allows us to map the PTEs WC. I've not done thorough testing or
> > > performance measurements with this patch, but it should be decent.
> > >
> > > This is based on a patch from Jesse with the original commit message
> > > > I've only lightly tested this so far, but the corruption seems to be
> > > > gone if I write the GFX_FLSH_CNTL reg after binding an object. This
> > > > register should control the TLB for the system agent, which is what CPU
> > > > mapped objects will go through.
> > >
> > > It has been updated for the new AGP-less code by me, and included with
> > > it is feedback from the original patch.
> > >
> > > CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > ---
> > > drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > > 2 files changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index 16fe960..e5f0a7f 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -391,6 +391,7 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
> > > }
> > >
> > > out:
> > > + I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> > > /* XXX: This serves as a posting read preserving the way the old code
> > > * works. It's not clear if this is strictly necessary or just voodoo
> > > * based on what I've tried to gather from the docs.
> > > @@ -593,8 +594,8 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > > if (ret)
> > > return ret;
> > >
> > > - dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
> > > - dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> > > + dev_priv->mm.gtt->gtt = ioremap_wc(gtt_bus_addr,
> > > + dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> > >
> > > /* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
> > > DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 1d54328..e8c578f 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -683,6 +683,8 @@
> > > #define CM0_RC_OP_FLUSH_DISABLE (1<<0)
> > > #define BB_ADDR 0x02140 /* 8 bytes */
> > > #define GFX_FLSH_CNTL 0x02170 /* 915+ only */
> > > +#define GFX_FLSH_CNTL_GEN6 0x101008
> > > +#define GFX_FLSH_CNTL_EN (1<<0)
> > > #define ECOSKPD 0x021d0
> > > #define ECO_GATING_CX_ONLY (1<<3)
> > > #define ECO_FLIP_DONE (1<<0)
> >
> > Looks good. Has anyone tried this on gen3-5?
>
> pre-gen6 still use the intel_gtt code, since we need to for the next
> decade or so to keep ums (and crappy xvmc on gen3) going.
Flush needs to happen somewhere though...
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 09/10] drm/i915: flush system agent TLBs on SNB
2012-10-23 14:35 ` Jesse Barnes
@ 2012-10-23 14:42 ` Ben Widawsky
0 siblings, 0 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 14:42 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Tue, 23 Oct 2012 07:35:48 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Tue, 23 Oct 2012 16:28:53 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > On Tue, Oct 23, 2012 at 07:17:53AM -0700, Jesse Barnes wrote:
> > > On Mon, 22 Oct 2012 18:34:14 -0700
> > > Ben Widawsky <ben@bwidawsk.net> wrote:
> > >
> > > > This allows us to map the PTEs WC. I've not done thorough
> > > > testing or performance measurements with this patch, but it
> > > > should be decent.
> > > >
> > > > This is based on a patch from Jesse with the original commit
> > > > message
> > > > > I've only lightly tested this so far, but the corruption
> > > > > seems to be gone if I write the GFX_FLSH_CNTL reg after
> > > > > binding an object. This register should control the TLB for
> > > > > the system agent, which is what CPU mapped objects will go
> > > > > through.
> > > >
> > > > It has been updated for the new AGP-less code by me, and
> > > > included with it is feedback from the original patch.
> > > >
> > > > CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> > > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > > > 2 files changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > > b/drivers/gpu/drm/i915/i915_gem_gtt.c index 16fe960..e5f0a7f
> > > > 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > > @@ -391,6 +391,7 @@ static void gen6_ggtt_bind_object(struct
> > > > drm_i915_gem_object *obj, }
> > > >
> > > > out:
> > > > + I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> > > > /* XXX: This serves as a posting read preserving the
> > > > way the old code
> > > > * works. It's not clear if this is strictly necessary
> > > > or just voodoo
> > > > * based on what I've tried to gather from the docs.
> > > > @@ -593,8 +594,8 @@ int i915_gem_gtt_init(struct drm_device
> > > > *dev) if (ret)
> > > > return ret;
> > > >
> > > > - dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
> > > > -
> > > > dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> > > > + dev_priv->mm.gtt->gtt = ioremap_wc(gtt_bus_addr,
> > > > +
> > > > dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> > > > /* GMADR is the PCI aperture used by SW to access
> > > > tiled GFX surfaces in a linear fashion. */
> > > > DRM_DEBUG_DRIVER("GMADR size = %dM\n",
> > > > dev_priv->mm.gtt->gtt_mappable_entries >> 8); diff --git
> > > > a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h index 1d54328..e8c578f 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h +++
> > > > b/drivers/gpu/drm/i915/i915_reg.h @@ -683,6 +683,8 @@ #define
> > > > CM0_RC_OP_FLUSH_DISABLE (1<<0) #define BB_ADDR
> > > > 0x02140 /* 8 bytes */ #define GFX_FLSH_CNTL 0x02170 /*
> > > > 915+ only */ +#define GFX_FLSH_CNTL_GEN6 0x101008
> > > > +#define GFX_FLSH_CNTL_EN (1<<0)
> > > > #define ECOSKPD 0x021d0
> > > > #define ECO_GATING_CX_ONLY (1<<3)
> > > > #define ECO_FLIP_DONE (1<<0)
> > >
> > > Looks good. Has anyone tried this on gen3-5?
> >
> > pre-gen6 still use the intel_gtt code, since we need to for the next
> > decade or so to keep ums (and crappy xvmc on gen3) going.
>
> Flush needs to happen somewhere though...
>
I thought pre-gen6 was working already?
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 09/10] drm/i915: flush system agent TLBs on SNB
2012-10-23 1:34 ` [PATCH 09/10] drm/i915: flush system agent TLBs on SNB Ben Widawsky
2012-10-23 14:17 ` Jesse Barnes
@ 2012-10-25 21:07 ` Jesse Barnes
2012-10-26 20:55 ` [PATCH 09/10 v2] " Ben Widawsky
2 siblings, 0 replies; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 21:07 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:14 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> This allows us to map the PTEs WC. I've not done thorough testing or
> performance measurements with this patch, but it should be decent.
>
> This is based on a patch from Jesse with the original commit message
> > I've only lightly tested this so far, but the corruption seems to be
> > gone if I write the GFX_FLSH_CNTL reg after binding an object. This
> > register should control the TLB for the system agent, which is what CPU
> > mapped objects will go through.
>
> It has been updated for the new AGP-less code by me, and included with
> it is feedback from the original patch.
>
> CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 16fe960..e5f0a7f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -391,6 +391,7 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
> }
>
> out:
> + I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> /* XXX: This serves as a posting read preserving the way the old code
> * works. It's not clear if this is strictly necessary or just voodoo
> * based on what I've tried to gather from the docs.
> @@ -593,8 +594,8 @@ int i915_gem_gtt_init(struct drm_device *dev)
> if (ret)
> return ret;
>
> - dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
> - dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> + dev_priv->mm.gtt->gtt = ioremap_wc(gtt_bus_addr,
> + dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
>
> /* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
> DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1d54328..e8c578f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -683,6 +683,8 @@
> #define CM0_RC_OP_FLUSH_DISABLE (1<<0)
> #define BB_ADDR 0x02140 /* 8 bytes */
> #define GFX_FLSH_CNTL 0x02170 /* 915+ only */
> +#define GFX_FLSH_CNTL_GEN6 0x101008
> +#define GFX_FLSH_CNTL_EN (1<<0)
> #define ECOSKPD 0x021d0
> #define ECO_GATING_CX_ONLY (1<<3)
> #define ECO_FLIP_DONE (1<<0)
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 09/10 v2] drm/i915: flush system agent TLBs on SNB
2012-10-23 1:34 ` [PATCH 09/10] drm/i915: flush system agent TLBs on SNB Ben Widawsky
2012-10-23 14:17 ` Jesse Barnes
2012-10-25 21:07 ` Jesse Barnes
@ 2012-10-26 20:55 ` Ben Widawsky
2 siblings, 0 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-26 20:55 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
This allows us to map the PTEs WC. I've not done thorough testing or
performance measurements with this patch, but it should be decent.
This is based on a patch from Jesse with the original commit message
> I've only lightly tested this so far, but the corruption seems to be
> gone if I write the GFX_FLSH_CNTL reg after binding an object. This
> register should control the TLB for the system agent, which is what CPU
> mapped objects will go through.
It has been updated for the new AGP-less code by me, and included with
it is feedback from the original patch.
v2: Updated to reflect paranoia on pte updates/register posting reads.
This entails doing the flush (with posting_read) after the pte readback
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by [v1]: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 11 +++++++++--
drivers/gpu/drm/i915/i915_reg.h | 2 ++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f832ab0..47a408a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -402,6 +402,13 @@ out:
* hardware should work, we must keep this posting read for paranoia.
*/
WARN_ON(readl(>t_entries[i-1]) != pte_encode(dev, addr, level));
+
+ /* This next bit makes the above posting read even more important. We
+ * want to flush the TLBs only after we're certain all the PTE updates
+ * have finished.
+ */
+ I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
+ POSTING_READ(GFX_FLSH_CNTL_GEN6);
}
void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
@@ -599,8 +606,8 @@ int i915_gem_gtt_init(struct drm_device *dev)
if (ret)
return ret;
- dev_priv->mm.gtt->gtt = ioremap(gtt_bus_addr,
- dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
+ dev_priv->mm.gtt->gtt = ioremap_wc(gtt_bus_addr,
+ dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
/* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
DRM_DEBUG_DRIVER("GMADR size = %dM\n", dev_priv->mm.gtt->gtt_mappable_entries >> 8);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1d54328..e8c578f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -683,6 +683,8 @@
#define CM0_RC_OP_FLUSH_DISABLE (1<<0)
#define BB_ADDR 0x02140 /* 8 bytes */
#define GFX_FLSH_CNTL 0x02170 /* 915+ only */
+#define GFX_FLSH_CNTL_GEN6 0x101008
+#define GFX_FLSH_CNTL_EN (1<<0)
#define ECOSKPD 0x021d0
#define ECO_GATING_CX_ONLY (1<<3)
#define ECO_FLIP_DONE (1<<0)
--
1.8.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 10/10] drm/i915: Kill off actually requiring AGP
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
` (8 preceding siblings ...)
2012-10-23 1:34 ` [PATCH 09/10] drm/i915: flush system agent TLBs on SNB Ben Widawsky
@ 2012-10-23 1:34 ` Ben Widawsky
2012-10-23 2:03 ` Ben Widawsky
2012-10-25 21:09 ` Jesse Barnes
2012-10-26 20:58 ` [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
10 siblings, 2 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 1:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Primarily for my own testing to make sure I actually killed off any
dependencies on AGP.
I'd happily extend this with CONFIG_ options or some such to make it
upstreamable if people were interested.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/Kconfig | 2 --
drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gem.c | 2 ++
drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
5 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 18321b68b..3afd84d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -119,8 +119,6 @@ config DRM_I810
config DRM_I915
tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
depends on DRM
- depends on AGP
- depends on AGP_INTEL
# we need shmfs for the swappable backing store, and in particular
# the shmem_readpage() which depends upon tmpfs
select SHMEM
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7837e5..cb5a9d8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt,
"Enable PPGTT (default: true)");
static struct drm_driver driver;
+#ifdef KEEP_AGP_DEPS
extern int intel_agp_enabled;
+#endif
#define INTEL_VGA_DEVICE(id, info) { \
.class = PCI_BASE_CLASS_DISPLAY << 16, \
@@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (intel_info->gen != 3) {
driver.driver_features &=
~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP);
+#ifdef KEEP_AGP_DEPS
} else if (!intel_agp_enabled) {
DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
return -ENODEV;
+#else
+ } else if (intel_info->gen < 6) {
+ DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
+ return -ENODEV;
+#endif
}
return drm_get_pci_dev(pdev, ent, &driver);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 534d282..b331d73 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev);
void i915_gem_gtt_fini(struct drm_device *dev);
extern inline void i915_gem_chipset_flush(struct drm_device *dev)
{
+#ifdef KEEP_AGP_DEPS
if (INTEL_INFO(dev)->gen < 6)
intel_gtt_chipset_flush();
+#endif
}
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ae3d4c1..2f4ce5d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev)
drm_i915_private_t *dev_priv = dev->dev_private;
int ret;
+#ifdef KEEP_AGP_DEPS
if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
return -EIO;
+#endif
if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1))
I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e5f0a7f..5097d0c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
obj->base.size >> PAGE_SHIFT);
}
+#ifdef KEEP_AGP_DEPS
/* XXX kill agp_type! */
static unsigned int cache_level_to_agp_type(struct drm_device *dev,
enum i915_cache_level cache_level)
@@ -282,6 +283,7 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev,
return AGP_USER_MEMORY;
}
}
+#endif
static bool do_idling(struct drm_i915_private *dev_priv)
{
@@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct drm_device *dev,
volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
+#ifdef KEEP_AGP_DEPS
if (INTEL_INFO(dev)->gen < 6) {
intel_gtt_clear_range(first_entry, num_entries);
return;
}
+#endif
if (WARN(num_entries > max_entries,
"First entry = %d; Num entries = %d (max=%d)\n",
@@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
if (INTEL_INFO(dev)->gen < 6) {
+#ifdef KEEP_AGP_DEPS
unsigned int agp_type = cache_level_to_agp_type(dev, cache_level);
intel_gtt_insert_sg_entries(obj->pages,
obj->gtt_space->start >> PAGE_SHIFT,
agp_type);
+#endif
} else {
gen6_ggtt_bind_object(obj, cache_level);
}
@@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
/* On modern platforms we need not worry ourself with the legacy
* hostbridge query stuff. Skip it entirely
*/
+#ifdef KEEP_AGP_DEPS
if (INTEL_INFO(dev)->gen < 6) {
ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
if (!ret) {
@@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
}
return 0;
}
-
+#endif
dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
if (!dev_priv->mm.gtt)
@@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
iounmap(dev_priv->mm.gtt->gtt);
teardown_scratch_page(dev);
+#ifdef KEEP_AGP_DEPS
if (INTEL_INFO(dev)->gen < 6)
intel_gmch_remove();
+#endif
kfree(dev_priv->mm.gtt);
}
--
1.7.12.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 10/10] drm/i915: Kill off actually requiring AGP
2012-10-23 1:34 ` [PATCH 10/10] drm/i915: Kill off actually requiring AGP Ben Widawsky
@ 2012-10-23 2:03 ` Ben Widawsky
2012-10-25 21:09 ` Jesse Barnes
1 sibling, 0 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-23 2:03 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
I mean to say this patch is RFC
On Mon, 22 Oct 2012 18:34:15 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> Primarily for my own testing to make sure I actually killed off any
> dependencies on AGP.
>
> I'd happily extend this with CONFIG_ options or some such to make it
> upstreamable if people were interested.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/Kconfig | 2 --
> drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_gem.c | 2 ++
> drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
> 5 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 18321b68b..3afd84d 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -119,8 +119,6 @@ config DRM_I810
> config DRM_I915
> tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
> depends on DRM
> - depends on AGP
> - depends on AGP_INTEL
> # we need shmfs for the swappable backing store, and in
> particular # the shmem_readpage() which depends upon tmpfs
> select SHMEM
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c index a7837e5..cb5a9d8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt,
> "Enable PPGTT (default: true)");
>
> static struct drm_driver driver;
> +#ifdef KEEP_AGP_DEPS
> extern int intel_agp_enabled;
> +#endif
>
> #define INTEL_VGA_DEVICE(id, info) { \
> .class = PCI_BASE_CLASS_DISPLAY << 16, \
> @@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent) if (intel_info->gen != 3) {
> driver.driver_features &=
> ~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP);
> +#ifdef KEEP_AGP_DEPS
> } else if (!intel_agp_enabled) {
> DRM_ERROR("drm/i915 can't work without intel_agp
> module!\n"); return -ENODEV;
> +#else
> + } else if (intel_info->gen < 6) {
> + DRM_ERROR("drm/i915 can't work without intel_agp
> module!\n");
> + return -ENODEV;
> +#endif
> }
>
> return drm_get_pci_dev(pdev, ent, &driver);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 534d282..b331d73 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev);
> void i915_gem_gtt_fini(struct drm_device *dev);
> extern inline void i915_gem_chipset_flush(struct drm_device *dev)
> {
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6)
> intel_gtt_chipset_flush();
> +#endif
> }
>
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c index ae3d4c1..2f4ce5d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev)
> drm_i915_private_t *dev_priv = dev->dev_private;
> int ret;
>
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
> return -EIO;
> +#endif
>
> if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1))
> I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c index e5f0a7f..5097d0c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct
> i915_hw_ppgtt *ppgtt, obj->base.size >> PAGE_SHIFT);
> }
>
> +#ifdef KEEP_AGP_DEPS
> /* XXX kill agp_type! */
> static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> enum i915_cache_level
> cache_level) @@ -282,6 +283,7 @@ static unsigned int
> cache_level_to_agp_type(struct drm_device *dev, return
> AGP_USER_MEMORY; }
> }
> +#endif
>
> static bool do_idling(struct drm_i915_private *dev_priv)
> {
> @@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct
> drm_device *dev, volatile void __iomem *gtt_base =
> dev_priv->mm.gtt->gtt + first_entry; const int max_entries =
> dev_priv->mm.gtt->gtt_total_entries - first_entry;
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6) {
> intel_gtt_clear_range(first_entry, num_entries);
> return;
> }
> +#endif
>
> if (WARN(num_entries > max_entries,
> "First entry = %d; Num entries = %d (max=%d)\n",
> @@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct
> drm_i915_gem_object *obj, {
> struct drm_device *dev = obj->base.dev;
> if (INTEL_INFO(dev)->gen < 6) {
> +#ifdef KEEP_AGP_DEPS
> unsigned int agp_type = cache_level_to_agp_type(dev,
> cache_level); intel_gtt_insert_sg_entries(obj->pages,
> obj->gtt_space->start >>
> PAGE_SHIFT, agp_type);
> +#endif
> } else {
> gen6_ggtt_bind_object(obj, cache_level);
> }
> @@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
> /* On modern platforms we need not worry ourself with the
> legacy
> * hostbridge query stuff. Skip it entirely
> */
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6) {
> ret = intel_gmch_probe(dev_priv->bridge_dev,
> dev->pdev, NULL); if (!ret) {
> @@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
> }
> return 0;
> }
> -
> +#endif
>
> dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt),
> GFP_KERNEL); if (!dev_priv->mm.gtt)
> @@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> iounmap(dev_priv->mm.gtt->gtt);
> teardown_scratch_page(dev);
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6)
> intel_gmch_remove();
> +#endif
> kfree(dev_priv->mm.gtt);
> }
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 10/10] drm/i915: Kill off actually requiring AGP
2012-10-23 1:34 ` [PATCH 10/10] drm/i915: Kill off actually requiring AGP Ben Widawsky
2012-10-23 2:03 ` Ben Widawsky
@ 2012-10-25 21:09 ` Jesse Barnes
2012-10-25 22:15 ` Ben Widawsky
1 sibling, 1 reply; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 21:09 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:15 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> Primarily for my own testing to make sure I actually killed off any
> dependencies on AGP.
>
> I'd happily extend this with CONFIG_ options or some such to make it
> upstreamable if people were interested.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/Kconfig | 2 --
> drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_gem.c | 2 ++
> drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
> 5 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 18321b68b..3afd84d 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -119,8 +119,6 @@ config DRM_I810
> config DRM_I915
> tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
> depends on DRM
> - depends on AGP
> - depends on AGP_INTEL
> # we need shmfs for the swappable backing store, and in particular
> # the shmem_readpage() which depends upon tmpfs
> select SHMEM
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a7837e5..cb5a9d8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt,
> "Enable PPGTT (default: true)");
>
> static struct drm_driver driver;
> +#ifdef KEEP_AGP_DEPS
> extern int intel_agp_enabled;
> +#endif
>
> #define INTEL_VGA_DEVICE(id, info) { \
> .class = PCI_BASE_CLASS_DISPLAY << 16, \
> @@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (intel_info->gen != 3) {
> driver.driver_features &=
> ~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP);
> +#ifdef KEEP_AGP_DEPS
> } else if (!intel_agp_enabled) {
> DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
> return -ENODEV;
> +#else
> + } else if (intel_info->gen < 6) {
> + DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
> + return -ENODEV;
> +#endif
> }
>
> return drm_get_pci_dev(pdev, ent, &driver);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 534d282..b331d73 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev);
> void i915_gem_gtt_fini(struct drm_device *dev);
> extern inline void i915_gem_chipset_flush(struct drm_device *dev)
> {
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6)
> intel_gtt_chipset_flush();
> +#endif
> }
>
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ae3d4c1..2f4ce5d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev)
> drm_i915_private_t *dev_priv = dev->dev_private;
> int ret;
>
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
> return -EIO;
> +#endif
>
> if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1))
> I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index e5f0a7f..5097d0c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
> obj->base.size >> PAGE_SHIFT);
> }
>
> +#ifdef KEEP_AGP_DEPS
> /* XXX kill agp_type! */
> static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> enum i915_cache_level cache_level)
> @@ -282,6 +283,7 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> return AGP_USER_MEMORY;
> }
> }
> +#endif
>
> static bool do_idling(struct drm_i915_private *dev_priv)
> {
> @@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct drm_device *dev,
> volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
> const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
>
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6) {
> intel_gtt_clear_range(first_entry, num_entries);
> return;
> }
> +#endif
>
> if (WARN(num_entries > max_entries,
> "First entry = %d; Num entries = %d (max=%d)\n",
> @@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
> {
> struct drm_device *dev = obj->base.dev;
> if (INTEL_INFO(dev)->gen < 6) {
> +#ifdef KEEP_AGP_DEPS
> unsigned int agp_type = cache_level_to_agp_type(dev, cache_level);
> intel_gtt_insert_sg_entries(obj->pages,
> obj->gtt_space->start >> PAGE_SHIFT,
> agp_type);
> +#endif
> } else {
> gen6_ggtt_bind_object(obj, cache_level);
> }
> @@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
> /* On modern platforms we need not worry ourself with the legacy
> * hostbridge query stuff. Skip it entirely
> */
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6) {
> ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
> if (!ret) {
> @@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
> }
> return 0;
> }
> -
> +#endif
>
> dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
> if (!dev_priv->mm.gtt)
> @@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> iounmap(dev_priv->mm.gtt->gtt);
> teardown_scratch_page(dev);
> +#ifdef KEEP_AGP_DEPS
> if (INTEL_INFO(dev)->gen < 6)
> intel_gmch_remove();
> +#endif
> kfree(dev_priv->mm.gtt);
> }
It would be cool to actually remove the dep for pre-gen6. I don't
think we need this patch in either case.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 10/10] drm/i915: Kill off actually requiring AGP
2012-10-25 21:09 ` Jesse Barnes
@ 2012-10-25 22:15 ` Ben Widawsky
2012-10-25 22:39 ` Jesse Barnes
0 siblings, 1 reply; 50+ messages in thread
From: Ben Widawsky @ 2012-10-25 22:15 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Thu, 25 Oct 2012 14:09:09 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 22 Oct 2012 18:34:15 -0700
> Ben Widawsky <ben@bwidawsk.net> wrote:
>
> > Primarily for my own testing to make sure I actually killed off any
> > dependencies on AGP.
> >
> > I'd happily extend this with CONFIG_ options or some such to make it
> > upstreamable if people were interested.
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> > drivers/gpu/drm/Kconfig | 2 --
> > drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
> > drivers/gpu/drm/i915/i915_drv.h | 2 ++
> > drivers/gpu/drm/i915/i915_gem.c | 2 ++
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
> > 5 files changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 18321b68b..3afd84d 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -119,8 +119,6 @@ config DRM_I810
> > config DRM_I915
> > tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
> > depends on DRM
> > - depends on AGP
> > - depends on AGP_INTEL
> > # we need shmfs for the swappable backing store, and in particular
> > # the shmem_readpage() which depends upon tmpfs
> > select SHMEM
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index a7837e5..cb5a9d8 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt,
> > "Enable PPGTT (default: true)");
> >
> > static struct drm_driver driver;
> > +#ifdef KEEP_AGP_DEPS
> > extern int intel_agp_enabled;
> > +#endif
> >
> > #define INTEL_VGA_DEVICE(id, info) { \
> > .class = PCI_BASE_CLASS_DISPLAY << 16, \
> > @@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > if (intel_info->gen != 3) {
> > driver.driver_features &=
> > ~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP);
> > +#ifdef KEEP_AGP_DEPS
> > } else if (!intel_agp_enabled) {
> > DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
> > return -ENODEV;
> > +#else
> > + } else if (intel_info->gen < 6) {
> > + DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
> > + return -ENODEV;
> > +#endif
> > }
> >
> > return drm_get_pci_dev(pdev, ent, &driver);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 534d282..b331d73 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev);
> > void i915_gem_gtt_fini(struct drm_device *dev);
> > extern inline void i915_gem_chipset_flush(struct drm_device *dev)
> > {
> > +#ifdef KEEP_AGP_DEPS
> > if (INTEL_INFO(dev)->gen < 6)
> > intel_gtt_chipset_flush();
> > +#endif
> > }
> >
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index ae3d4c1..2f4ce5d 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev)
> > drm_i915_private_t *dev_priv = dev->dev_private;
> > int ret;
> >
> > +#ifdef KEEP_AGP_DEPS
> > if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
> > return -EIO;
> > +#endif
> >
> > if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1))
> > I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000);
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index e5f0a7f..5097d0c 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
> > obj->base.size >> PAGE_SHIFT);
> > }
> >
> > +#ifdef KEEP_AGP_DEPS
> > /* XXX kill agp_type! */
> > static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> > enum i915_cache_level cache_level)
> > @@ -282,6 +283,7 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> > return AGP_USER_MEMORY;
> > }
> > }
> > +#endif
> >
> > static bool do_idling(struct drm_i915_private *dev_priv)
> > {
> > @@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct drm_device *dev,
> > volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
> > const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
> >
> > +#ifdef KEEP_AGP_DEPS
> > if (INTEL_INFO(dev)->gen < 6) {
> > intel_gtt_clear_range(first_entry, num_entries);
> > return;
> > }
> > +#endif
> >
> > if (WARN(num_entries > max_entries,
> > "First entry = %d; Num entries = %d (max=%d)\n",
> > @@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
> > {
> > struct drm_device *dev = obj->base.dev;
> > if (INTEL_INFO(dev)->gen < 6) {
> > +#ifdef KEEP_AGP_DEPS
> > unsigned int agp_type = cache_level_to_agp_type(dev, cache_level);
> > intel_gtt_insert_sg_entries(obj->pages,
> > obj->gtt_space->start >> PAGE_SHIFT,
> > agp_type);
> > +#endif
> > } else {
> > gen6_ggtt_bind_object(obj, cache_level);
> > }
> > @@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > /* On modern platforms we need not worry ourself with the legacy
> > * hostbridge query stuff. Skip it entirely
> > */
> > +#ifdef KEEP_AGP_DEPS
> > if (INTEL_INFO(dev)->gen < 6) {
> > ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
> > if (!ret) {
> > @@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > }
> > return 0;
> > }
> > -
> > +#endif
> >
> > dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
> > if (!dev_priv->mm.gtt)
> > @@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev)
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > iounmap(dev_priv->mm.gtt->gtt);
> > teardown_scratch_page(dev);
> > +#ifdef KEEP_AGP_DEPS
> > if (INTEL_INFO(dev)->gen < 6)
> > intel_gmch_remove();
> > +#endif
> > kfree(dev_priv->mm.gtt);
> > }
>
> It would be cool to actually remove the dep for pre-gen6. I don't
> think we need this patch in either case.
>
Your linker is going to complain without it, unless I don't follow your
meaning.
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 10/10] drm/i915: Kill off actually requiring AGP
2012-10-25 22:15 ` Ben Widawsky
@ 2012-10-25 22:39 ` Jesse Barnes
2012-10-25 22:47 ` Daniel Vetter
0 siblings, 1 reply; 50+ messages in thread
From: Jesse Barnes @ 2012-10-25 22:39 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, 25 Oct 2012 15:15:02 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> On Thu, 25 Oct 2012 14:09:09 -0700
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>
> > On Mon, 22 Oct 2012 18:34:15 -0700
> > Ben Widawsky <ben@bwidawsk.net> wrote:
> >
> > > Primarily for my own testing to make sure I actually killed off any
> > > dependencies on AGP.
> > >
> > > I'd happily extend this with CONFIG_ options or some such to make it
> > > upstreamable if people were interested.
> > >
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > ---
> > > drivers/gpu/drm/Kconfig | 2 --
> > > drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
> > > drivers/gpu/drm/i915/i915_drv.h | 2 ++
> > > drivers/gpu/drm/i915/i915_gem.c | 2 ++
> > > drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
> > > 5 files changed, 22 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index 18321b68b..3afd84d 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -119,8 +119,6 @@ config DRM_I810
> > > config DRM_I915
> > > tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
> > > depends on DRM
> > > - depends on AGP
> > > - depends on AGP_INTEL
> > > # we need shmfs for the swappable backing store, and in particular
> > > # the shmem_readpage() which depends upon tmpfs
> > > select SHMEM
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index a7837e5..cb5a9d8 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt,
> > > "Enable PPGTT (default: true)");
> > >
> > > static struct drm_driver driver;
> > > +#ifdef KEEP_AGP_DEPS
> > > extern int intel_agp_enabled;
> > > +#endif
> > >
> > > #define INTEL_VGA_DEVICE(id, info) { \
> > > .class = PCI_BASE_CLASS_DISPLAY << 16, \
> > > @@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > if (intel_info->gen != 3) {
> > > driver.driver_features &=
> > > ~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP);
> > > +#ifdef KEEP_AGP_DEPS
> > > } else if (!intel_agp_enabled) {
> > > DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
> > > return -ENODEV;
> > > +#else
> > > + } else if (intel_info->gen < 6) {
> > > + DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
> > > + return -ENODEV;
> > > +#endif
> > > }
> > >
> > > return drm_get_pci_dev(pdev, ent, &driver);
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 534d282..b331d73 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev);
> > > void i915_gem_gtt_fini(struct drm_device *dev);
> > > extern inline void i915_gem_chipset_flush(struct drm_device *dev)
> > > {
> > > +#ifdef KEEP_AGP_DEPS
> > > if (INTEL_INFO(dev)->gen < 6)
> > > intel_gtt_chipset_flush();
> > > +#endif
> > > }
> > >
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index ae3d4c1..2f4ce5d 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev)
> > > drm_i915_private_t *dev_priv = dev->dev_private;
> > > int ret;
> > >
> > > +#ifdef KEEP_AGP_DEPS
> > > if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
> > > return -EIO;
> > > +#endif
> > >
> > > if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1))
> > > I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000);
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index e5f0a7f..5097d0c 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
> > > obj->base.size >> PAGE_SHIFT);
> > > }
> > >
> > > +#ifdef KEEP_AGP_DEPS
> > > /* XXX kill agp_type! */
> > > static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> > > enum i915_cache_level cache_level)
> > > @@ -282,6 +283,7 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev,
> > > return AGP_USER_MEMORY;
> > > }
> > > }
> > > +#endif
> > >
> > > static bool do_idling(struct drm_i915_private *dev_priv)
> > > {
> > > @@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct drm_device *dev,
> > > volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
> > > const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
> > >
> > > +#ifdef KEEP_AGP_DEPS
> > > if (INTEL_INFO(dev)->gen < 6) {
> > > intel_gtt_clear_range(first_entry, num_entries);
> > > return;
> > > }
> > > +#endif
> > >
> > > if (WARN(num_entries > max_entries,
> > > "First entry = %d; Num entries = %d (max=%d)\n",
> > > @@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
> > > {
> > > struct drm_device *dev = obj->base.dev;
> > > if (INTEL_INFO(dev)->gen < 6) {
> > > +#ifdef KEEP_AGP_DEPS
> > > unsigned int agp_type = cache_level_to_agp_type(dev, cache_level);
> > > intel_gtt_insert_sg_entries(obj->pages,
> > > obj->gtt_space->start >> PAGE_SHIFT,
> > > agp_type);
> > > +#endif
> > > } else {
> > > gen6_ggtt_bind_object(obj, cache_level);
> > > }
> > > @@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > > /* On modern platforms we need not worry ourself with the legacy
> > > * hostbridge query stuff. Skip it entirely
> > > */
> > > +#ifdef KEEP_AGP_DEPS
> > > if (INTEL_INFO(dev)->gen < 6) {
> > > ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
> > > if (!ret) {
> > > @@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > > }
> > > return 0;
> > > }
> > > -
> > > +#endif
> > >
> > > dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
> > > if (!dev_priv->mm.gtt)
> > > @@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev)
> > > struct drm_i915_private *dev_priv = dev->dev_private;
> > > iounmap(dev_priv->mm.gtt->gtt);
> > > teardown_scratch_page(dev);
> > > +#ifdef KEEP_AGP_DEPS
> > > if (INTEL_INFO(dev)->gen < 6)
> > > intel_gmch_remove();
> > > +#endif
> > > kfree(dev_priv->mm.gtt);
> > > }
> >
> > It would be cool to actually remove the dep for pre-gen6. I don't
> > think we need this patch in either case.
> >
>
> Your linker is going to complain without it, unless I don't follow your
> meaning.
I mean unless we split the driver into gen6+ and pre-gen6 (with a
config option or otherwise), we can't really kill the dependency unless
we actually pull the rest of the AGP stuff we need into i915. I'd kind
of like to do the latter, but the non-KMS configs would break.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 10/10] drm/i915: Kill off actually requiring AGP
2012-10-25 22:39 ` Jesse Barnes
@ 2012-10-25 22:47 ` Daniel Vetter
2012-10-26 20:52 ` Ben Widawsky
0 siblings, 1 reply; 50+ messages in thread
From: Daniel Vetter @ 2012-10-25 22:47 UTC (permalink / raw)
To: Jesse Barnes; +Cc: Ben Widawsky, intel-gfx
On Thu, Oct 25, 2012 at 03:39:08PM -0700, Jesse Barnes wrote:
> > > It would be cool to actually remove the dep for pre-gen6. I don't
> > > think we need this patch in either case.
> > >
> >
> > Your linker is going to complain without it, unless I don't follow your
> > meaning.
>
> I mean unless we split the driver into gen6+ and pre-gen6 (with a
> config option or otherwise), we can't really kill the dependency unless
> we actually pull the rest of the AGP stuff we need into i915. I'd kind
> of like to do the latter, but the non-KMS configs would break.
Also, some kms config actually break because we've managed to ship
userspace that uses kms, but in parts also still uses the legacy drmMap
api. Which in our case means we need to have a working (but fake) agp
driver around. This entire thing is one giant mess, but I think with the
gen6+ stuff moved it's about as good as it will get. And I don't mind the
old crap too much, we have an entire dungeon full of them in i915_dma.c,
too ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 10/10] drm/i915: Kill off actually requiring AGP
2012-10-25 22:47 ` Daniel Vetter
@ 2012-10-26 20:52 ` Ben Widawsky
0 siblings, 0 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-26 20:52 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Fri, 26 Oct 2012 00:47:31 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Oct 25, 2012 at 03:39:08PM -0700, Jesse Barnes wrote:
> > > > It would be cool to actually remove the dep for pre-gen6. I
> > > > don't think we need this patch in either case.
> > > >
> > >
> > > Your linker is going to complain without it, unless I don't
> > > follow your meaning.
> >
> > I mean unless we split the driver into gen6+ and pre-gen6 (with a
> > config option or otherwise), we can't really kill the dependency
> > unless we actually pull the rest of the AGP stuff we need into
> > i915. I'd kind of like to do the latter, but the non-KMS configs
> > would break.
>
> Also, some kms config actually break because we've managed to ship
> userspace that uses kms, but in parts also still uses the legacy
> drmMap api. Which in our case means we need to have a working (but
> fake) agp driver around. This entire thing is one giant mess, but I
> think with the gen6+ stuff moved it's about as good as it will get.
> And I don't mind the old crap too much, we have an entire dungeon
> full of them in i915_dma.c, too ;-)
> -Daniel
The idea (as discussed on IRC) was to provide something like a
CONFIG_GEN6 which will properly detect and error out for older
platforms. This would allow embedded systems to save on the AGP size
when they are trying to store everything in flash.
Anyway, for now, I think we all agree that this is unnecessary, but if
someone building an embedded system with i915 graphics wants to save
roughly 500k of binary data size (and some smaller amount of runtime
memory)...
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 00/10] Kill AGP dependencies for Gen6+
2012-10-23 1:34 [PATCH 00/10] Kill AGP dependencies for Gen6+ Ben Widawsky
` (9 preceding siblings ...)
2012-10-23 1:34 ` [PATCH 10/10] drm/i915: Kill off actually requiring AGP Ben Widawsky
@ 2012-10-26 20:58 ` Ben Widawsky
10 siblings, 0 replies; 50+ messages in thread
From: Ben Widawsky @ 2012-10-26 20:58 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Mon, 22 Oct 2012 18:34:05 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> As a result of some work which I can't yet talk about, it became
> obvious that now was the time to kill AGP. As this is actually prep
> work for the interesting stuff but the interesting stuff can't be
> released yet, I serve it up here for review now.
>
> Of the 10 patches, only 1 is really interesting as far as review is
> concerned (6).
>
> 1-5 is prep, and things which I could sneak in.
> 6 is moving all the necessary bits from agp layer into i915
> 7 is a small fix introduced separately from 6 for bisectability
> 8 is removing all the cruft
> 9 is a patch from Jesse, rebased (requested by Daniel)
> 10 was done for my testing, and also is RFC
>
>
> Ben Widawsky (10):
> drm/i915:
> http://cgit.freedesktop.org/~bwidawsk/drm-intel/log/?h=kill-agpNo
> LLC_MLC for HSW. drm/i915: Add dev to ppgtt drm/i915: introduce
> gtt_pte_tdrm/i915: Extract PPGTT pte encoding
> drm/i915: move more pte encoding to pte encode
> drm/i915: Stop using AGP layer for GEN6+
> drm/i915: Calculate correct stolen size for GEN7+
> drm/i915: Kill off now unused gen6+ AGP code
> drm/i915: flush system agent TLBs on SNB
> drm/i915: Kill off actually requiring AGP
>
> drivers/char/agp/intel-agp.h | 91 ---------
> drivers/char/agp/intel-gtt.c | 309
> +----------------------------
> drivers/gpu/drm/Kconfig | 2 -
> drivers/gpu/drm/i915/i915_dma.c | 16 +-
> drivers/gpu/drm/i915/i915_drv.c | 8 +
> drivers/gpu/drm/i915/i915_drv.h | 13 +-
> drivers/gpu/drm/i915/i915_gem.c | 14 +-
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 308
> ++++++++++++++++++++++++----
> drivers/gpu/drm/i915/i915_reg.h | 10 +
> include/drm/intel-gtt.h | 7 +- 11 files changed,
> 317 insertions(+), 463 deletions(-)
>
Daniel, I've pushed the updated branch with all of the reviewed-bys
that I have here (and dropped patch 10):
http://cgit.freedesktop.org/~bwidawsk/drm-intel/log/?h=kill-agp
Assuming nobody has issue with the v2 I've posted of patches 6 and 9, I
think it's ready for merge to -testing/-queued.
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 50+ messages in thread