<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>v3 with typo fixes and additional comments/questions..<br>
</p>
<br>
<div style="color: rgb(0, 0, 0);">
<div>
<div id="divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Arial,Helvetica,sans-serif">
<br>
<div style="color:rgb(0,0,0)">
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" color="#000000" face="Calibri, sans-serif"><b>From:</b> Bridgman, John<br>
<b>Sent:</b> December 11, 2016 10:21 PM<br>
<b>To:</b> Dave Airlie; Wentland, Harry<br>
<b>Cc:</b> dri-devel; amd-gfx mailing list; 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>
<div id="divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Arial,Helvetica,sans-serif">
<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 anything else...</p>
<p><br>
</p>
<p><font size="2"><span style="font-size:10pt">>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.</span></font><br>
</p>
<p><br>
</p>
<p>Can we say "restructuring" just for consistency with Daniel's message (the HW-dependent bits don't need to be rewritten but the way they are used/called needs to change) ?<br>
</p>
<p><br>
</p>
<p><font size="2"><span style="font-size:10pt">>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.</span></font><br>
</p>
<p><br>
</p>
<p>That's fair. There was an implicit "when it's suitable" assumption in the RFC, but we'll make that explicit in the future.<br>
</p>
<p><font size="2"><span style="font-size:10pt"></span></font></p>
<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 is only used for 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>
</div>
</div>
</div>
</div>
</div>
</div>
</body>
</html>