Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] The newly added pixel_blend_mode property is not initialized in the 
plane's reset hook, leading to an invalid initial atomic state.
--

commit dea256106aa6a4944ed84de30a43b82c973ff72f
Author: Thierry Reding <[email protected]>

drm/tegra: Add blend mode properties

The default programming in the driver matches the "coverage" blend mode,
so add the corresponding pixel blend mode property to let userspace know
about it.

> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 0b3fcc7011b35..fefc3761a4bc7 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -939,6 +940,7 @@ static struct drm_plane 
> *tegra_primary_plane_create(struct drm_device *drm,
>       }
>  
>       drm_plane_helper_add(&plane->base, &tegra_plane_helper_funcs);
> +     drm_plane_create_blend_mode_property(&plane->base, blend_caps);

[Severity: High]
Since tegra_plane_reset() in drivers/gpu/drm/tegra/plane.c uses kzalloc_obj()
to zero-allocate the atomic plane state:

        state = kzalloc_obj(*state);
        if (state) {
                plane->state = &state->base;
                plane->state->plane = plane;

Will this cause atomic commits to fail with -EINVAL if userspace performs a
commit without changing the blend mode?

The pixel_blend_mode field is implicitly left at 0 (DRM_MODE_BLEND_PREMULTI).
Because only DRM_MODE_BLEND_COVERAGE (enum value 1) is exposed in blend_caps,
the core DRM enum validator (drm_property_change_valid_get()) will reject
the 0 value when userspace queries and subsequently commits the state.

Should tegra_plane_reset() be updated to explicitly initialize
pixel_blend_mode to DRM_MODE_BLEND_COVERAGE?

>       drm_plane_create_zpos_property(&plane->base, plane->index, 0, 255);

[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to