Re: [PATCH 07/12] drm/amdgpu: implement cgs gpu memory callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 30.09.2015 08:54, Daniel Vetter wrote:
On Tue, Sep 29, 2015 at 04:28:13PM -0400, Alex Deucher wrote:
On Tue, Sep 29, 2015 at 4:10 PM, Dave Airlie <airlied@xxxxxxxxx> wrote:
On 30 September 2015 at 01:41, Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
On Tue, Sep 29, 2015 at 11:20 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
On Tue, Sep 29, 2015 at 02:39:49PM +0200, Christian König wrote:
On 29.09.2015 13:40, Daniel Vetter wrote:
On Thu, Jul 09, 2015 at 12:21:06PM -0400, Alex Deucher wrote:
From: Chunming Zhou <david1.zhou@xxxxxxx>

This implements the cgs interface for allocating
GPU memory.

Reviewed-by: Jammy Zhou <Jammy.Zhou@xxxxxxx>
I don't see that review anywhere on a m-l ... where is it?
Jammy reviewed the stuff internally before we made it public, that's why you
can't find it.
Can we get these reviews done publically? it's kinda hard to know how
well someone
reviewed things if we have no external copy. Like did Jammy a) read
the patch, and
slap Reviewed-by on it, or did he b) comment on some whitespace issues
and slap R-b
on it, or c) did he suggest a bunch of changes and those changes were
made and a new
version was produced and he r-b'ed that etc.

We are pushing for that and making steady progress, but things are
slow to move internally.

The other stuff seems a lot more benign. For the irq abstraction
specifically it might be worth looking at the irq_chip stuff linux core
has, which is what's used to virtualize/abstract irq routing and handling.
But for that stuff it's a balance thing really how much you reinvent
wheels internally in the driver (e.g. i915 has it's own power_well stuff
which is pretty much just powerdomains reinvented, with less features).

I think that's one of the hardest things in the kernel: finding out if
a solution already exists or not.  We implemented our own version of
mfd for our ACP audio block driver.  Upon upsteaming we were alerted
to mfd's existence and we converted the driver to use mfd.  At the end
of the day it was a lot of work for minimal gain, at least from a
functionality perspective.  I wish we had known about it sooner.  I'll
take a look at the irq_chip stuff.  Thanks for the heads up!
You say for minimal gain, but this is pretty much going to keep happening
to you with the development model you have chosen, get used to rewriting
things you consider finished and reviewed. I've said it before so I'll use this
to reiterate, your patches are only starting the process when you post them,
all the internal stuff you do is nice and all but it could all be done
externally
if you guys weren't so stuck on internal IP review. Otherwise you
should be taking
into account that this overhead will continue to exist in all your development,
and adjust schedules to suit.
We do take that into account as evidenced by the multiple revisions of
the ACP patch set for example.  We know there may be a delta between
short term deliverables and what eventually goes upstream and we take
that into account.  That doesn't change the overall amount of work
involved.  The fact is we didn't know about mfd so we didn't use it.
I don't see how we could have avoided rewriting it if we didn't know
about it in the first place.  When we sent the patches out, we found
out about it and made the appropriate changes.  My point was just that
we aren't the only ones this happens to.
Discussing early designs on irc helps a lot with that. But ime irc is one
step further away from just dragging engineers onto the mailing list, and
discussing new stuff on irc before patches get written instead of just
review is one step more. That still leaves you with the problem with
knowing whom to talk to, but for modularization we have at least Thierry
Reding and Laurent Pinchart with a lot of soc experience outside of drm,
they tend to know what's out there.

Yeah, completely agree.

Alex and I have spend a lot of time and effort with developers previously working on closed source code to enable them to contribute to the different open source projects.

Anybody who did something like that before knows that it certainly needs time for certain concepts to sink in. Especially thinks like stable interface and certain design criteria seem to be hard to get accepted.

So guys feel to criticize the code whenever you think it make sense and please do so as early as possible, e.g. on the first round of patches not after ten revisions.

We in turn try to get the code out as soon as possible, without keeping it stuck internally for too long.

Christian.

-Daniel

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux