Virtualization - Dec 2021 - [PATCH] drm: Return error codes from struct drm_driver.gem_create_object

On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann
wrote:
> GEM helper libraries use struct drm_driver.gem_create_object to let
> drivers override GEM object allocation. On failure, the call returns
> NULL.
> 
> Change the semantics to make the calls return a pointer-encoded error.
> This aligns the callback with its callers. Fixes the ingenic driver,
> which already returns an error pointer.
> 
> Also update the callers to handle the involved types more strictly.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
> There is an alternative patch at [1] that updates the value returned
> by ingenics' gem_create_object to NULL. Fixing the interface to return
> an errno code is more consistent with the rest of the GEM functions.
> 
> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147 at kili/ 
My fix was already applied and backported to -stable etc...  Your
patch is not developed against a current tree so you broke it.

That's the tricky thing with changing the API because say people wrote
their code last week where returning NULL was correct.  When they submit
their driver upstream, everything will merge and build but it will break
at runtime.

For now, it's only vc4_create_object() which is broken.

regards,
dan carpenter

Hi

Am 06.12.21 um 11:42 schrieb Dan Carpenter:
> On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
>> GEM helper libraries use struct drm_driver.gem_create_object to let
>> drivers override GEM object allocation. On failure, the call returns
>> NULL.
>>
>> Change the semantics to make the calls return a pointer-encoded error.
>> This aligns the callback with its callers. Fixes the ingenic driver,
>> which already returns an error pointer.
>>
>> Also update the callers to handle the involved types more strictly.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>> There is an alternative patch at [1] that updates the value returned
>> by ingenics' gem_create_object to NULL. Fixing the interface to
return
>> an errno code is more consistent with the rest of the GEM functions.
>>
>> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147 at kili/
> 
> My fix was already applied and backported to -stable etc...  Your
> patch is not developed against a current tree so you broke it.
Do you have a specific link? I just checked the stable tree at [1] and 
there no trace of your patch.

Patches for DRM should go through through DRM trees; drm-misc-fixes in 
this case. Exceptions should at least be announce on dri-devel. Neither 
is the case here.

Best regards
Thomas

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> 
> That's the tricky thing with changing the API because say people wrote
> their code last week where returning NULL was correct.  When they submit
> their driver upstream, everything will merge and build but it will break
> at runtime.
> 
> For now, it's only vc4_create_object() which is broken.
> 
> regards,
> dan carpenter
> 
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 N?rnberg, Germany
(HRB 36809, AG N?rnberg)
Gesch?ftsf?hrer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20211206/0b37dd52/attachment.sig>