On Thu, Sep 2, 2021 at 1:52 AM Liu, Monk <monk....@amd.com> wrote: > > [AMD Official Use Only] > > >>> > I'm not sure I can add much to help this along, I'm sure Alex has some > internal training, > Once your driver is upstream, it belongs to upstream, you can maintain it, > but you no longer control it 100%, it's a tradeoff, it's not one companies > always understand. > Usually people are fine developing away internally, but once interaction with > other parts of the kernel/subsystem is required they have the realisation > that they needed to work upstream 6 months earlier. > The best time to interact with upstream was 6 months ago, the second best > time is now. > <<< > > Daniel/AlexD > > I didn't mean your changes on AMD driver need my personal approval or review > ... and I'm totally already get used that our driver is not 100% under > control by AMDers, > but supposedly any one from community (including you) who tend to change > AMD's driver need at least to get approvement from someone in AMD, e.g.: > AlexD or Christian, doesn't that reasonable? > just like we need your approve if we try to modify DRM-sched, or need > panfrost's approval if we need to change panfrost code ... > > by only CC AMD's engineers looks not quite properly, how do you know if your > changes (on AMD code part) are conflicting with AMD's on-going internal > features/refactoring or not ? >
We keep as up to date as possible with upstream. I don't have the bandwidth to verify every patch, but in most cases I try and look at them. In your first example, the patch basically just adds a new parameter to some common functions. Drivers that don't need that parameter don't use it. It shouldn't really affect the functionality. There are lots of changes that touch our driver that we are largely not aware of. E.g., APIs that we may use may change the function signatures with no intended functional changes. If problems are found they are reported and resolved. It is a collective effort. If there are changes that would conflict with stuff we are doing in our tree we should bring them up when the relevant patches are being discussed. We can also make changes to core functionality like scheduler, ttm, etc. that would affect other drivers. When we send out the patches we cc the relevant maintainers, but ultimately the ones who participate in the discussion set the direction. That's why participation is important. Alex > Thanks > > ------------------------------------------ > Monk Liu | Cloud-GPU Core team > ------------------------------------------ > > -----Original Message----- > From: Dave Airlie <airl...@gmail.com> > Sent: Thursday, September 2, 2021 2:51 AM > To: Alex Deucher <alexdeuc...@gmail.com> > Cc: Liu, Monk <monk....@amd.com>; Daniel Vetter <dan...@ffwll.ch>; Koenig, > Christian <christian.koe...@amd.com>; Grodzovsky, Andrey > <andrey.grodzov...@amd.com>; Chen, JingWen <jingwen.ch...@amd.com>; DRI > Development <dri-devel@lists.freedesktop.org>; amd-...@lists.freedesktop.org > Subject: Re: [diagnostic TDR mode patches] unify our solution > opinions/suggestions in one thread > > On Thu, 2 Sept 2021 at 01:20, Alex Deucher <alexdeuc...@gmail.com> wrote: > > > > On Wed, Sep 1, 2021 at 6:19 AM Liu, Monk <monk....@amd.com> wrote: > > > > > > [AMD Official Use Only] > > > > > > Daniel > > > > > > From the link you share it looks you(or someone else) have quite a bunch > > > patches that changes DRM_SCHED or even amdgpu, by that case before they > > > are merged to kernel tree I'm wondering if any AMD develop reviewed them ? > > > > > > They looks to me somehow conflicting with what we changed in our repo.... > > > > > > It is really a chaos for AMDer if someone else out side of AMD changes > > > our kernel driver (or/and scheduler) without reviewed by AMDer, just like > > > we are requiring your review if we tend to change scheduler's logic here > > > .... > > > > > > This one changes AMD's code: > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo > > > re.kernel.org%2Fdri-devel%2F20210625133327.2598825-2-boris.brezillon > > > %40collabora.com%2F&data=04%7C01%7CMonk.Liu%40amd.com%7C6c507d18 > > > d65341ef53bb08d96d7976e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0% > > > 7C637661190727875969%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ > > > QIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=BWJSkKN > > > y2%2BwjxbQrfxGPzuJ5PBpBwB4aV0ZH6QoJGEg%3D&reserved=0 > > > And I didn't see any reviewed-by from AMDers ... > > > > > > This one also touches AMD's code: > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo > > > re.kernel.org%2Fdri-devel%2F20200604081224.863494-12-daniel.vetter%4 > > > 0ffwll.ch%2F&data=04%7C01%7CMonk.Liu%40amd.com%7C6c507d18d65341e > > > f53bb08d96d7976e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63766 > > > 1190727885929%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2 > > > luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=%2F8vIVXCWjHkM > > > 56pcYI9EvuzhbsZhV9WczkKaBJE67KQ%3D&reserved=0 > > > Which is conflicting with one patch we submitted (in our repo > > > rightnow), and neither see AMDder gave a review-by on this one (let > > > me know if I missed it) > > > > > > > Monk, this is not how upstream works. You need to participate. > > That's how communities work. There's a reason all these discussions > > happen on public mailing lists. The patch author can't be expected to > > know every person on every vendor team to CC with a patch. If you > > have concerns, you need to raise them when the patches are being > > discussed. > > > > I'm not sure I can add much to help this along, I'm sure Alex has some > internal training, > > Once your driver is upstream, it belongs to upstream, you can maintain it, > but you no longer control it 100%, it's a tradeoff, it's not one companies > always understand. > > Usually people are fine developing away internally, but once interaction with > other parts of the kernel/subsystem is required they have the realisation > that they needed to work upstream 6 months earlier. > > The best time to interact with upstream was 6 months ago, the second best > time is now. > > Dave.
