Re: [PATCH v2 1/2] drm/file: Rehabilitate the firstopen hook for non-legacy drivers

[Paul Kocialkowski]

Hi,

On Fri, 2019-03-29 at 09:09 +0000, Daniel Stone wrote:
> Hi,
> 
> On Thu, 28 Mar 2019 at 18:53, Daniel Vetter <daniel@xxxxxxxx> wrote:
> > On Thu, Mar 21, 2019 at 04:27:06PM +0100, Paul Kocialkowski wrote:
> > > I don't see other options either, and using firstclose/lastopen feels
> > > overall more readable in the driver code.
> > > 
> > > I'm not sure there is such a big overhead associated with allocating
> > > the binner BO (it seems that the current implementation tries to alloc
> > > until the specific memory constraints for the buffer are met, so
> > > perhaps that can take time). But if there is, I suppose it's best to
> > > have that when starting up rather than delaying the first render
> > > operation.
> > 
> > I'm not entirely buying the "we don't need this for fbcon only" argument -
> > there's plenty of dumb kms clients too (boot splash and whatever else
> > there might be). If you don't want to keep this around I think allocating
> > on first non-dumb bo allocation and dropping it when the last such fd
> > closes sounds like a much better idea. Needs a bit more state, you need to
> > track per drm_file whether you've already allocated a non-dumb bo, and a
> > drm_device refcount, but that's not much. Firstopen feels like the wrong
> > thing.
> > 
> > Another option would be first_renderopen or something like that, except
> > you can also render on the legacy node and I'm not sure how much there's a
> > demand for this in other drivers. In the end you have open/close
> > callbacks, you can do all the driver specific things you want to do in
> > there.
> 
> I'd like to avoid doing it in open where possible, since that hurts
> device enumeration from userspace.

I've noticed the same issue with firstopen, where our buffer is
allocated/liberated a couple of times during enumeration, before the
final open that stays alive during use.

I'm not sure what is preferable between that and allocating when the
GPU is first used. Slowing down the first GPU operation with the
allocation does not sound too great either and it feels like the buffer
should have been allocated earlier.

To me, it feels I think it's better to have delays due to allocation at
enumeration / startup rather than later on, but I might be missing some
elements to have a clear idea.

What do you think?

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com