On Wed, 21 Mar 2012 22:19:36 +0100
Daniel Vetter <daniel at ffwll.ch> wrote:
> On Wed, Mar 21, 2012 at 12:48:38PM -0700, Jesse Barnes wrote:
> > The GT can snoop CPU writes, but doesn't snoop into the CPU cache when
> > it does writes, so we can't use the cache bits the same way.
> >
> > So map the status and pipe control pages as uncached on ValleyView, and
> > only set the pages to cached if we're on a supported platform.
> >
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 2 +
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 35 ++++++++++++++++++++++++------
> > 2 files changed, 30 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index e4fa294..a636703 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -255,6 +255,7 @@ static const struct intel_device_info intel_valleyview_m_info = {
> > .has_bsd_ring = 1,
> > .has_blt_ring = 1,
> > .is_valleyview = 1,
> > + .has_llc = 0,
> > };
> >
> > static const struct intel_device_info intel_valleyview_d_info = {
> > @@ -264,6 +265,7 @@ static const struct intel_device_info intel_valleyview_d_info = {
> > .has_bsd_ring = 1,
> > .has_blt_ring = 1,
> > .is_valleyview = 1,
> > + .has_llc = 0,
>
> Usually we don't set feature bits to 0, please drop these 2 hunks.
>
> > };
> >
> > static const struct pci_device_id pciidlist[] = { /* aka */
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index ca3972f..f52abc4 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -319,6 +319,8 @@ init_pipe_control(struct intel_ring_buffer *ring)
> > {
> > struct pipe_control *pc;
> > struct drm_i915_gem_object *obj;
> > + int cache_level = HAS_LLC(ring->dev) ? I915_CACHE_LLC : I915_CACHE_NONE;
> > + struct drm_device *dev;
>
> Nope, that's not gonna work. Afaik we have 3 kinds of snoopable bus access
> from the gpu:
> - llc, i.e. snoopable access for all render operations, support if
> intel_info->has_llc == 1
> - snoopable access to untiled mem with the blitter, supported on all
> generations (down to mighty old i81x)
> - snoopable access to the hw status page
>
> Please clear up the confusion here. Below you also use the IS_VLV macro,
> that seems more appropriate. Also I'm wondering whether this is supposed
> to be fixed in future silicon revisions, if so please mark this as a w/a
> that can be reaped as soon as we don't use this early silicon for testing
> any more.
Yeah I don't like this either. I have a meeting with the hw guys on
Friday and will try to clear this up.
It may be best to just check for IS_VALLEYVIEW here instead (this is
what I had before, then I decided to get clever so I could squash in
the HAS_LLC change in i915_gem.c). You're right there are multiple
types of cacheability we need to accommodate.
So maybe HAS_LLC for i915_gem.c and HWS_CACHEABLE or something to
easily identify the retro chips...
--
Jesse Barnes, Intel Open Source Technology Center
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120321/bd0fea99/attachment.pgp>
