Hi Monk,
Am 02.09.21 um 07:52 schrieb Liu, Monk:
[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?
I'm fearing that just repeating what Alex said, but to make it clear
once more: That is *not* necessary!
The shared repository is owned by upstream maintainers and they are
usually free to do restructuring work without getting acknowledge from
every single driver maintainer.
Anybody can of course technically object to upstream design decisions,
but that means that you need to pay attention to the mailing lists in
the first place.
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 ?
Well because AMD is supposed to work in public as much as possible and
ask upstream before doing changes to the code base.
Additional to that design decisions are supposed to be discussed on the
mailing list and *not* internally.
Regards,
Christian.
Thanks
------------------------------------------
Monk Liu | Cloud-GPU Core team
------------------------------------------
-----Original Message-----
From: Dave Airlie <airlied@xxxxxxxxx>
Sent: Thursday, September 2, 2021 2:51 AM
To: Alex Deucher <alexdeucher@xxxxxxxxx>
Cc: Liu, Monk <Monk.Liu@xxxxxxx>; Daniel Vetter <daniel@xxxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Grodzovsky, Andrey <Andrey.Grodzovsky@xxxxxxx>; Chen, JingWen <JingWen.Chen2@xxxxxxx>; DRI Development <dri-devel@xxxxxxxxxxxxxxxxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [diagnostic TDR mode patches] unify our solution opinions/suggestions in one thread
On Thu, 2 Sept 2021 at 01:20, Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
On Wed, Sep 1, 2021 at 6:19 AM Liu, Monk <Monk.Liu@xxxxxxx> 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.
