[RFC PATCH] drm/panfrost: Add initial panfrost driver

[Alyssa Rosenzweig]

> bitmasks in the kernel use unsigned long arrays. A strange choice
> which I guess was either because it predated 64-bit or enables atomic
> ops which tend to be on the native size. So this just fixes the size
> to 64-bits for 32 and 64 bit systems.

Bizarre, but if that's the standard, then OK.

> Issues I trimmed down to only the kernel ones. Features were small
> enough, I just left them all.

+1

> Job slots, yes. I think I have a define somewhere already I'll use.

NUM_JOB_SLOTS 

> I'll just point out that freedreno is also a single address space
> (though there are patches for that now).

Huh. That seems.... huh.

> Systems simply don't have enough RAM that you'd want to use 4G for
> graphics memory.

Fair enough.

> That reminds me, I want to not start at 0 to catch NULL addresses.

+1

> They aren't exposed outside of the driver, so namespacing them is a
> bit pointless and just makes the names too long. I will at least
> rename base_hw_feature.

+1

> Unlike the issues list, it was small enough I didn't really think about it here.

+1 though this may change in the future.

> If this is ever going to be used in commercial products, it will need
> to be supported whether in tree or out. We can leave that for another
> day.

*angry acceptance noises*

> Primarily to enforce good hygiene to only access a sub-block's
> registers within its .c file.
> 
> > Copyright/origin is more transparent that way too.
> 
> Good point. Otherwise, I can say "Register definitions from ...,
> Copyright ARM..."

Perhaps it would make sense to have a regs/ folder with a
.h corresponding to each of the components. So it's separated out but
still prevents unintended access (since you'd need to #include the wrong
component explicitly, and that'd be immediately vetoed on a patch).

> This means I just stuffed in fixed values to these registers. The
> kbase driver uses hw_issues and such to determine the setup.

Oh, I see. 

> Hard to say with the context deleted...

Sorry! I'm new to this, bear with me :)

> As powering on h/w takes time, a delay seemed appropriate. i think I
> can check some status bits though.

That conceptually seems better, I think.

> IMO, if someone wants to improve the documentation here, that can come later.

I'd be happy to submit a patch helping with this (either before we
upstream or after).

> Sadly, I confirmed it is sitting on my desk. Samsung chromebook
> (snow).

Any chance that was an earlier dev version of Snow that you got from
your employer and this was maybe fixed in the real thing? Or would that
be too convenient?

> Hard coded ATM.

No, like, what does "affinity" mean in this context? I didn't study
kbase too hard so I'm having troubles following any of this.

> Tell me what you'd like it to be...

Maybe some nice round number like 64 or 128..? Kidding ;)

> Isn't it obvious?
> 
> It's a TODO we need to figure out.

...Ah. Yes, that should have been obvious. My apologies.

> Maybe? Reloading the module will reset things.

The issue is that unprivileged userspace apps are able to submit
arbitrarily crazy workloads to the kernel (without necessarily needing
the driver's permission). It's trivial for unprivileged code to cause a
fault (and thanks to driver bugs, WebGL is an attack surface here as
well); doing so should not lock up the GPU requiring superuser access to
correct. Faults are not catastrophic but they do happen.

> Only matters for bifrost. There for a TODO I guess.

Gotcha, alright.

> That's what the kbase driver does...

Huh.

> That affects more than just the code here. We probably need a higher
> level TODO list.

+1

-Alyssa