* [PATCH] drm/i915: Rename GFX_TLB_INVALIDATE_ALWAYS
@ 2014-03-21 17:18 Chris Wilson
2014-03-22 3:29 ` Gupta, Sourab
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2014-03-21 17:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Gupta, Sourab
The documentation calls this GFX_MODE bit "Flush TLB invalidate Mode".
However, that is not a good name for an enable bit as it doesn't make it
clear what is enabled. An even worse name is GFX_TLB_INVALIDATE_ALWAYS
as enabling that bit actually prevents the TLB from being invalidated at
every flush. This leads to great confusion when reading code and
proposed patches. To get around this try to bake in what is enabled by
setting the bit and call it GFX_TLB_INVALIDATE_EXPLICIT.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: "Gupta, Sourab" <sourab.gupta@intel.com>
---
Alternatively, we could call it GFX_TLB_INVALIDATE_MANUAL.
---
drivers/gpu/drm/i915/i915_reg.h | 2 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9476b483417a..cd5ac2e03c7e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -843,7 +843,7 @@ enum punit_power_well {
#define GFX_MODE_GEN7 0x0229c
#define RING_MODE_GEN7(ring) ((ring)->mmio_base+0x29c)
#define GFX_RUN_LIST_ENABLE (1<<15)
-#define GFX_TLB_INVALIDATE_ALWAYS (1<<13)
+#define GFX_TLB_INVALIDATE_EXPLICIT (1<<13)
#define GFX_SURFACE_FAULT_ENABLE (1<<12)
#define GFX_REPLAY_MODE (1<<11)
#define GFX_PSMI_GRANULARITY (1<<10)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 99b1d8e15711..9470e51f0c9f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -595,11 +595,11 @@ static int init_render_ring(struct intel_ring_buffer *ring)
/* Required for the hardware to program scanline values for waiting */
if (INTEL_INFO(dev)->gen == 6)
I915_WRITE(GFX_MODE,
- _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_ALWAYS));
+ _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
if (IS_GEN7(dev))
I915_WRITE(GFX_MODE_GEN7,
- _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) |
+ _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
_MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
if (INTEL_INFO(dev)->gen >= 5) {
@@ -622,7 +622,7 @@ static int init_render_ring(struct intel_ring_buffer *ring)
* TODO: consider explicitly setting the bit for GEN5
*/
ring->itlb_before_ctx_switch =
- !!(I915_READ(GFX_MODE) & GFX_TLB_INVALIDATE_ALWAYS);
+ !!(I915_READ(GFX_MODE) & GFX_TLB_INVALIDATE_EXPLICIT);
}
if (INTEL_INFO(dev)->gen >= 6)
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915: Rename GFX_TLB_INVALIDATE_ALWAYS
2014-03-21 17:18 [PATCH] drm/i915: Rename GFX_TLB_INVALIDATE_ALWAYS Chris Wilson
@ 2014-03-22 3:29 ` Gupta, Sourab
2014-03-22 11:23 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Gupta, Sourab @ 2014-03-22 3:29 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx@lists.freedesktop.org
On Fri, 2014-03-21 at 17:18 +0000, Chris Wilson wrote:
> The documentation calls this GFX_MODE bit "Flush TLB invalidate Mode".
> However, that is not a good name for an enable bit as it doesn't make it
> clear what is enabled. An even worse name is GFX_TLB_INVALIDATE_ALWAYS
> as enabling that bit actually prevents the TLB from being invalidated at
> every flush. This leads to great confusion when reading code and
> proposed patches. To get around this try to bake in what is enabled by
> setting the bit and call it GFX_TLB_INVALIDATE_EXPLICIT.
>
This looks fine. It clears the confusion around TLB INVALIDATE Bit.
We'll be developing our patch on top of this and sending the full series
of WA patches for VLV.
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: "Gupta, Sourab" <sourab.gupta@intel.com>
> ---
> Alternatively, we could call it GFX_TLB_INVALIDATE_MANUAL.
> ---
> drivers/gpu/drm/i915/i915_reg.h | 2 +-
> drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9476b483417a..cd5ac2e03c7e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -843,7 +843,7 @@ enum punit_power_well {
> #define GFX_MODE_GEN7 0x0229c
> #define RING_MODE_GEN7(ring) ((ring)->mmio_base+0x29c)
> #define GFX_RUN_LIST_ENABLE (1<<15)
> -#define GFX_TLB_INVALIDATE_ALWAYS (1<<13)
> +#define GFX_TLB_INVALIDATE_EXPLICIT (1<<13)
> #define GFX_SURFACE_FAULT_ENABLE (1<<12)
> #define GFX_REPLAY_MODE (1<<11)
> #define GFX_PSMI_GRANULARITY (1<<10)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 99b1d8e15711..9470e51f0c9f 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -595,11 +595,11 @@ static int init_render_ring(struct intel_ring_buffer *ring)
> /* Required for the hardware to program scanline values for waiting */
> if (INTEL_INFO(dev)->gen == 6)
> I915_WRITE(GFX_MODE,
> - _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_ALWAYS));
> + _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>
> if (IS_GEN7(dev))
> I915_WRITE(GFX_MODE_GEN7,
> - _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) |
> + _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
> _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>
> if (INTEL_INFO(dev)->gen >= 5) {
> @@ -622,7 +622,7 @@ static int init_render_ring(struct intel_ring_buffer *ring)
> * TODO: consider explicitly setting the bit for GEN5
> */
> ring->itlb_before_ctx_switch =
> - !!(I915_READ(GFX_MODE) & GFX_TLB_INVALIDATE_ALWAYS);
> + !!(I915_READ(GFX_MODE) & GFX_TLB_INVALIDATE_EXPLICIT);
> }
>
> if (INTEL_INFO(dev)->gen >= 6)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915: Rename GFX_TLB_INVALIDATE_ALWAYS
2014-03-22 3:29 ` Gupta, Sourab
@ 2014-03-22 11:23 ` Daniel Vetter
2014-03-23 8:50 ` Gupta, Sourab
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2014-03-22 11:23 UTC (permalink / raw)
To: Gupta, Sourab; +Cc: intel-gfx@lists.freedesktop.org
On Sat, Mar 22, 2014 at 03:29:11AM +0000, Gupta, Sourab wrote:
> On Fri, 2014-03-21 at 17:18 +0000, Chris Wilson wrote:
> > The documentation calls this GFX_MODE bit "Flush TLB invalidate Mode".
> > However, that is not a good name for an enable bit as it doesn't make it
> > clear what is enabled. An even worse name is GFX_TLB_INVALIDATE_ALWAYS
> > as enabling that bit actually prevents the TLB from being invalidated at
> > every flush. This leads to great confusion when reading code and
> > proposed patches. To get around this try to bake in what is enabled by
> > setting the bit and call it GFX_TLB_INVALIDATE_EXPLICIT.
> >
>
> This looks fine. It clears the confusion around TLB INVALIDATE Bit.
> We'll be developing our patch on top of this and sending the full series
> of WA patches for VLV.
btw if you think the patch is useful and you've reviewed it according to
the "Reviewer's Statement of Oversight" [1] then please always supply your
reviewed-by tag. Otherwise it's just the informal acked-by.
Patch merged into dinq.
Thanks, Daniel
[1] https://www.kernel.org/doc/Documentation/SubmittingPatches
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Rename GFX_TLB_INVALIDATE_ALWAYS
2014-03-22 11:23 ` Daniel Vetter
@ 2014-03-23 8:50 ` Gupta, Sourab
0 siblings, 0 replies; 4+ messages in thread
From: Gupta, Sourab @ 2014-03-23 8:50 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx@lists.freedesktop.org
On Sat, 2014-03-22 at 11:23 +0000, Daniel Vetter wrote:
> On Sat, Mar 22, 2014 at 03:29:11AM +0000, Gupta, Sourab wrote:
> > On Fri, 2014-03-21 at 17:18 +0000, Chris Wilson wrote:
> > > The documentation calls this GFX_MODE bit "Flush TLB invalidate Mode".
> > > However, that is not a good name for an enable bit as it doesn't make it
> > > clear what is enabled. An even worse name is GFX_TLB_INVALIDATE_ALWAYS
> > > as enabling that bit actually prevents the TLB from being invalidated at
> > > every flush. This leads to great confusion when reading code and
> > > proposed patches. To get around this try to bake in what is enabled by
> > > setting the bit and call it GFX_TLB_INVALIDATE_EXPLICIT.
> > >
> >
> > This looks fine. It clears the confusion around TLB INVALIDATE Bit.
> > We'll be developing our patch on top of this and sending the full series
> > of WA patches for VLV.
>
> btw if you think the patch is useful and you've reviewed it according to
> the "Reviewer's Statement of Oversight" [1] then please always supply your
> reviewed-by tag. Otherwise it's just the informal acked-by.
>
> Patch merged into dinq.
>
> Thanks, Daniel
>
Hi Daniel,
Thanks for pointing out. This patch looks good to me.
Reviewed by: Sourab Gupta <sourab.gupta@intel.com>
>
> [1] https://www.kernel.org/doc/Documentation/SubmittingPatches
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-23 8:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-21 17:18 [PATCH] drm/i915: Rename GFX_TLB_INVALIDATE_ALWAYS Chris Wilson
2014-03-22 3:29 ` Gupta, Sourab
2014-03-22 11:23 ` Daniel Vetter
2014-03-23 8:50 ` Gupta, Sourab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox