<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Arial,Helvetica,sans-serif;" dir="ltr">
<p>Thanks Dave. Apologies in advance for top posting but I'm stuck on a mail client that makes a big mess when I try...</p>
<font size="2"><span style="font-size:10pt;"></span></font>
<p><font size="2"><span style="font-size:10pt;"><br>
</span></font></p>
<p><font size="2"><span style="font-size:10pt;">>If DC was ready for the next-gen GPU it would be ready for the current<br>
>GPU, it's not the specific ASIC code that is the problem, it's the<br>
>huge midlayer sitting in the middle.</span></font><br>
</p>
<br>
<p>We realize that (a) we are getting into the high-risk-of-breakage part of the rework and (b) no matter how much we change the code structure there's a good chance that a month after it goes upstream one of us is going to find that more structural changes
are required. <br>
</p>
<p><br>
</p>
<p>I was kinda thinking that if we are doing high-risk activities (risk of subtle breakage not obvious regression, and/or risk of making structural changes that turn out to be a bad idea even though we all thought they were correct last week) there's an argument
for doing it in code which only supports cards that people can't buy yet.<br>
</p>
<br>
<div style="color: rgb(0, 0, 0);">
<div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" color="#000000" face="Calibri, sans-serif"><b>From:</b> Dave Airlie <airlied@gmail.com><br>
<b>Sent:</b> December 11, 2016 9:57 PM<br>
<b>To:</b> Wentland, Harry<br>
<b>Cc:</b> dri-devel; amd-gfx mailing list; Bridgman, John; Deucher, Alexander; Lazare, Jordan; Cheng, Tony; Cyr, Aric; Grodzovsky, Andrey<br>
<b>Subject:</b> Re: [RFC] Using DC in amdgpu for upcoming GPU</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt;">
<div class="PlainText">On 8 December 2016 at 12:02, Harry Wentland <harry.wentland@amd.com> wrote:<br>
> We propose to use the Display Core (DC) driver for display support on<br>
> AMD's upcoming GPU (referred to by uGPU in the rest of the doc). In order to<br>
> avoid a flag day the plan is to only support uGPU initially and transition<br>
> to older ASICs gradually.<br>
<br>
[FAQ: from past few days]<br>
<br>
1) Hey you replied to Daniel, you never addressed the points of the RFC!<br>
I've read it being said that I hadn't addressed the RFC, and you know<br>
I've realised I actually had, because the RFC is great but it<br>
presupposes the codebase as designed can get upstream eventually, and<br>
I don't think it can. The code is too littered with midlayering and<br>
other problems, that actually addressing the individual points of the<br>
RFC would be missing the main point I'm trying to make.<br>
<br>
This code needs rewriting, not cleaning, not polishing, it needs to be<br>
split into its constituent parts, and reintegrated in a form more<br>
Linux process friendly.<br>
<br>
I feel that if I reply to the individual points Harry has raised in<br>
this RFC, that it means the code would then be suitable for merging,<br>
which it still won't, and I don't want people wasting another 6<br>
months.<br>
<br>
If DC was ready for the next-gen GPU it would be ready for the current<br>
GPU, it's not the specific ASIC code that is the problem, it's the<br>
huge midlayer sitting in the middle.<br>
<br>
2) We really need to share all of this code between OSes, why does<br>
Linux not want it?<br>
<br>
Sharing code is a laudable goal and I appreciate the resourcing<br>
constraints that led us to the point at which we find ourselves, but<br>
the way forward involves finding resources to upstream this code,<br>
dedicated people (even one person) who can spend time on a day by day<br>
basis talking to people in the open and working upstream, improving<br>
other pieces of the drm as they go, reading atomic patches and<br>
reviewing them, and can incrementally build the DC experience on top<br>
of the Linux kernel infrastructure. Then having the corresponding<br>
changes in the DC codebase happen internally to correspond to how the<br>
kernel code ends up looking. Lots of this code overlaps with stuff the<br>
drm already does, lots of is stuff the drm should be doing, so patches<br>
to the drm should be sent instead.<br>
<br>
3) Then how do we upstream it?<br>
Resource(s) need(s) to start concentrating at splitting this thing up<br>
and using portions of it in the upstream kernel. We don't land fully<br>
formed code in the kernel if we can avoid it. Because you can't review<br>
the ideas and structure as easy as when someone builds up code in<br>
chunks and actually develops in the Linux kernel. This has always<br>
produced better more maintainable code. Maybe the result will end up<br>
improving the AMD codebase as well.<br>
<br>
4) Why can't we put this in staging?<br>
People have also mentioned staging, Daniel has called it a dead end,<br>
I'd have considered staging for this code base, and I still might.<br>
However staging has rules, and the main one is code in staging needs a<br>
TODO list, and agreed criteria for exiting staging, I don't think we'd<br>
be able to get an agreement on what the TODO list should contain and<br>
how we'd ever get all things on it done. If this code ended up in<br>
staging, it would most likely require someone dedicated to recreating<br>
it in the mainline driver in an incremental fashion, and I don't see<br>
that resource being available.<br>
<br>
5) Why is a midlayer bad?<br>
I'm not going to go into specifics on the DC midlayer, but we abhor<br>
midlayers for a fair few reasons. The main reason I find causes the<br>
most issues is locking. When you have breaks in code flow between<br>
multiple layers, but having layers calling back into previous layers<br>
it becomes near impossible to track who owns the locking and what the<br>
current locking state is.<br>
<br>
Consider<br>
drma -> dca -> dcb -> drmb<br>
drmc -> dcc -> dcb -> drmb<br>
<br>
We have two codes paths that go back into drmb, now maybe drma has a<br>
lock taken, but drmc doesn't, but we've no indication when we hit drmb<br>
of what the context pre entering the DC layer is. This causes all<br>
kinds of problems. The main requirement is the driver maintains the<br>
execution flow as much as possible. The only callback behaviour should<br>
be from an irq or workqueue type situations where you've handed<br>
execution flow to the hardware to do something and it is getting back<br>
to you. The pattern we use to get our of this sort of hole is helper<br>
libraries, we structure code as much as possible as leaf nodes that<br>
don't call back into the parents if we can avoid it (we don't always<br>
succeed).<br>
<br>
So the above might becomes<br>
drma-> dca_helper<br>
-> dcb_helper<br>
-> drmb.<br>
<br>
In this case the code flow is controlled by drma, dca/dcb might be<br>
modifying data or setting hw state but when we get to drmb it's easy<br>
to see what data is needs and what locking.<br>
<br>
DAL/DC goes against this in so many ways, and when I look at the code<br>
I'm never sure where to even start pulling the thread to unravel it.<br>
<br>
Some questions I have for AMD engineers that also I'd want to see<br>
addressed before any consideration of merging would happen!<br>
<br>
How do you plan on dealing with people rewriting or removing code<br>
upstream that is redundant in the kernel, but required for internal<br>
stuff?<br>
How are you going to deal with new Linux things that overlap<br>
incompatibly with your internally developed stuff?<br>
If the code is upstream will it be tested in the kernel by some QA<br>
group, or will there be some CI infrastructure used to maintain and to<br>
watch for Linux code that breaks assumptions in the DC code?<br>
Can you show me you understand that upstream code is no longer 100% in<br>
your control and things can happen to it that you might not expect and<br>
you need to deal with it?<br>
<br>
Dave.<br>
</div>
</span></font></div>
</div>
</body>
</html>