[PATCH 0/6] accel/ivpu: Changes for 6.15 2025-02-04
Maxime Ripard
mripard at kernel.org
Wed Feb 12 12:59:10 UTC 2025
On Wed, Feb 12, 2025 at 11:20:46AM +0100, Thomas Zimmermann wrote:
> Hi,
>
> here's a complaint about the lack of process and documentation in accel/,
> and ivpu specifically. I came across this series while preparing the weekly
> PR for drm-misc-next and found myself unable to extract much useful
> information to report. This is a problem for a development process that
> relies on transparency, accountability and collaboration. Other problematic
> examples are at [1] and [2]. IIRC I had similar issues in previous
> development cycles.
>
> I cannot assess the quality of the code itself, but the process and
> documentation involved does not meet the requirements.
>
> - 'Changes for <version>' is not an meaningful description for a patch
> series. It's not the submitter (or anyone else) deciding that this series
> gets merged into version so-and-so. The series gets merged when it is ready
> to be merged.
>
> - Apparently this series contains 3 different things (buffer imports,
> locking, debugging); so it should be 3 series with each addressing one of
> these topics.
>
> - The series' description just restates the patch descriptions briefly. It
> should rather give some indication of the problem being solved by the
> contained patches, and context on why this is worth solving. (I know that
> this is often complicated to state clearly to outsiders.)
>
> - Review should be public. I understand that it's often only one dev team
> working on a specific driver, discussing issues internally. Still it makes
> sense to do the code reviews in public, so that others can follow what is
> going on in the driver. Public code reviews are also necessary to establish
> consent and institutional knowledge within the wider developer community.
> You miss that with internal reviews.
>
> - These patches come with R-b tags pre-applied. Even for trivial changes,
> R-b tags should given in public. If the R-bs have been given elsewhere,
> please include a reference to that location. The tags (R-b, A-b, T-b, etc)
> are not just for verifying the code itself. They also establish trust in the
> development process involving each patch; and in the developers involved in
> that process. This needs to happen in public to be effective.
>
> - The kernel's (or any FOSS') development is organized around individuals,
> not organizations. Having each developer send their changes individually
> would likely resolve most of the current problems.
>
> I understand that accel is not graphics and can feel somewhat detached from
> the rest of DRM. Yet it is part of the DRM subsystem. This development
> cycles' ivpu series' made me go to IRC and ask for accel/ to be removed from
> the drm-misc tree. Luckily the other maintainer were more charitable. So I
> make these remarks in good faith and hope that we can improve the processes
> within accel/.
I'd also want to emphatize that the last three items at least are
applicable to the entire kernel, so even if they had a different tree,
it would still not be ok.
Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 273 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250212/1a669a81/attachment.sig>
More information about the dri-devel
mailing list