On 3/16/26 3:02 AM, Thomas Zimmermann wrote:
> Hi
>
> Am 16.03.26 um 00:49 schrieb David Lechner:
>> On 3/11/26 5:10 AM, Thomas Zimmermann wrote:
>>> Replace simple-display helpers with regular atomic helpers. Store the
>>> pipeline elements in struct st7586_device and initialize them as part
>>> of probing the device. Use mipi-dbi's existing helpers and initializer
>>> macros where possible.
>>>
>>> Effectively open-codes the modesetting code in the initializer helpers
>>> of mipi-dbi and simple-display. St7586 requires custom helpers for
>>> various pipeline elements, and non-freeing cleanup of the pipeline.
>>>
>>
>> ...
>>
>>> +static void st7586_plane_helper_atomic_update(struct drm_plane *plane,
>>> + struct drm_atomic_state *state)
>>> {
>>> - struct drm_plane_state *state = pipe->plane.state;
>>> - struct drm_shadow_plane_state *shadow_plane_state =
>>> to_drm_shadow_plane_state(state);
>>> - struct drm_framebuffer *fb = state->fb;
>>> + struct drm_plane_state *plane_state = plane->state;
>>> + struct drm_shadow_plane_state *shadow_plane_state =
>>> to_drm_shadow_plane_state(plane_state);
>>> + struct drm_framebuffer *fb = plane_state->fb;
>>> + struct drm_plane_state *old_plane_state =
>>> drm_atomic_get_old_plane_state(state, plane);
>>> struct drm_rect rect;
>>> int idx;
>>> - if (!pipe->crtc.state->active)
>>> + if (!fb)
>>> return;
>>> - if (!drm_dev_enter(fb->dev, &idx))
>>> - return;
>> What was wrong with returning early here?
>
> Nothing. Putting the affected code into a single block just seems to make the
> code better structured. But we can go back to the old style, if preferred.
I have a slight preference for avoiding more indentation when we can
help it.
>
> Best regards
> Thomas
>
>>
>>> + if (drm_dev_enter(plane->dev, &idx)) {
>>> + if (drm_atomic_helper_damage_merged(old_plane_state, plane_state,
>>> &rect))
>>> + st7586_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
>>> + &shadow_plane_state->fmtcnv_state);
>>> + drm_dev_exit(idx);
>>> + }
>>> +}
>>> - if (drm_atomic_helper_damage_merged(old_state, state, &rect))
>>> - st7586_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
>>> - &shadow_plane_state->fmtcnv_state);
>
