Re: [PATCH v5 0/7] drm: Reuse temporary memory for format conversion

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2486 bytes --]

Hi

Am 17.11.23 um 10:34 schrieb Maxime Ripard:
> On Mon, Oct 09, 2023 at 04:06:29PM +0200, Thomas Zimmermann wrote:
>> DRM's format-conversion helpers require temporary memory. Pass the
>> buffer from the caller to allow the caller to preallocate the buffer
>> memory.
>>
>> The motivation for this patchset is the recent work on a DRM panic
>> handler. [1] The panic handler requires format conversion to display an
>> error to the screen. But allocating memory during a kernel panic is
>> fragile. The changes in this patchset enable the DRM panic handler to
>> preallocate buffer storage before the panic occurs.
>>
>> Patch 1 adds struct drm_format_conv_state, a simple interface to pass
>> around the buffer storage. Patch 2 adds an instance of the struct to
>> the shadow-plane state. Patch 3 moves the buffer's memory management
>> from the format helpers into their callers within the DRM drivers. Most
>> of the affected drivers use the state instance stored in their shadow-
>> plane state. The shadow-plane code releases the buffer memory automatically.
>>
>> Patches 4 to 7 update three drivers to pre-allocate the format-conversion
>> buffer in their plane's atomic_check function. The drivers thus detect OOM
>> errors before the display update begins.
>>
>> Tested with simpledrm.
> 
> So, I just discovered that you merged that series.
> 
> You've complained before about "sneaking patches in", and while I was
> disagreeing with you then, this particular instance is definitely a
> strong case for it. You've merged it without telling anyone, and despite
> our ongoing conversation on the v4 that was active more recently than
> the v5. And that you never responded to.
> 
> Awesome.

My apologies. From my point of view, that conversion had ended. I left 
the patch set for a while to wait for further comments or questions, but 
nothing happened. So I merged it.

Revert it if you cannot live with the changes. IIRC you found the 
reduced number of alloc/free cycles to be irrelevant. But even then, the 
patches allow us to move the allocation from atomic_update to 
atomic_check, thus detecting allocation failures early. That's an 
improvement to me.

Best regards
Thomas

> 
> Maxime

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]