Re: [PATCH 2/2] drm/i915: Add simulator's host bridge

[Ben Widawsky <ben@bwidawsk.net>]

On Sat, 21 Jul 2012 10:42:13 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Fri, 20 Jul 2012 10:43:30 -0700, Ben Widawsky <ben@bwidawsk.net>
> wrote:
> > Add the host bridge ID used by the simulator. This was added in a
> > previous patch for the agp layer, but wasn't preserved here.  It
> > also gives us an opportunity to let the rest of the driver know
> > we're running as the simulator for various workarounds.
> 
> I like how minimal this looks, though it does worry me that is a
> little too easy... (Not least the question why the simulator doesn't
> work with forcewake... Doesn't that strike you as odd that we have
> rc6 bugs and the simulator dies... ;-)

At some point I should go through the simulator code... Actually it's a
bit complicated, but the forcewake handshake doesn't normally even make
it to the simulator. I'll follow up with you on irc if you care
further than that. An added bonus though is register accesses are quite
slow, and removing these is in itself quite beneficial (not measured).

> 
> I think I would rather see this as an intel_info so that it can be
> expanded upon simply for future/past simulators.

I tried to go this path. The immediate hack, and problem is I can't
detect the simulator until we probe the host bridge. All the INTEL_INFO
stuff is const. I tried INTEL_INFO(dev)->is_simulator=true. Now the
issue with setting up real structures for simulators... Instead I
propose we want to treat the simulator transparently as much as
possible. The only entirely unavoidable bit I've found so far is the
host bridge PCI id, and in fact it was the case at least until
forcewake on Haswell. I'd like to model the simulator as a quirky
platform instead of a quirky GPU. If we go the INTEL_INFO path we end
up with a simulator variant of each generation (analogous to
is_mobile/desktop) which I see as just superflous typing.

> 
> However, the fundamental question is do we ever ship simulators? If we
> try to avoid carrying pre-production w/a, shouldn't that mean that we
> don't even contemplate pushing simulator interfaces.

I am certain nobody would say we will *never* ship a simulator. We have
internal customers though that benefit from this.

> 
> Having said, carrying these patches centrally does mean that will be
> reviewed and kept in mind for future iterations.

Yes, more importantly, we can clone a repo, build it, and it will
just run.

> -Chris
> 

Thank you for reviewing.


-- 
Ben Widawsky, Intel Open Source Technology Center