Op 01-03-2019 om 14:13 schreef Laurent Pinchart:
> Hi Maarten,
>
> Thank you for the patch.
>
> On Fri, Mar 01, 2019 at 01:56:22PM +0100, Maarten Lankhorst wrote:
>> Convert rcar-du to using __drm_atomic_helper_crtc_reset(), instead of
>> writing its own version. Instead of open coding destroy_state(), call
>> it directly for freeing the old state.
> I don't think the second sentence applies to this patch.
>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>> Cc: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
>> Cc: linux-renesas-soc@xxxxxxxxxxxxxxx
>> ---
>> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 11 +++--------
>> 1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> index 4cdea14d552f..7766551e67fc 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> @@ -891,22 +891,17 @@ static void rcar_du_crtc_cleanup(struct drm_crtc *crtc)
>>
>> static void rcar_du_crtc_reset(struct drm_crtc *crtc)
>> {
>> - struct rcar_du_crtc_state *state;
>> + struct rcar_du_crtc_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
>>
>> - if (crtc->state) {
>> + if (crtc->state)
>> rcar_du_crtc_atomic_destroy_state(crtc, crtc->state);
>> - crtc->state = NULL;
>> - }
>>
>> - state = kzalloc(sizeof(*state), GFP_KERNEL);
>> + __drm_atomic_helper_crtc_reset(crtc, &state->state);
> state may be NULL here if the above kzalloc() failed. Let's keep the
> original order of the function, and simply call
> __drm_atomic_helper_crtc_reset() after the NULL check below.
There were 10 different ways crtc was implemented, I felt it was good to settle on one.
We don't handle during reset at all, would need to start propagating this first before we should handle errors, imho.
Looking more closely, it's the same way that errors in rcar_du_plane_reset() are handled. :)
Cheers,
~Maarten
