From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 049CEC2B9F4 for ; Tue, 22 Jun 2021 19:10:11 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B30E4601FE for ; Tue, 22 Jun 2021 19:10:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B30E4601FE Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3020A6E876; Tue, 22 Jun 2021 19:10:10 +0000 (UTC) Received: from mx2.smtp.larsendata.com (mx2.smtp.larsendata.com [91.221.196.228]) by gabe.freedesktop.org (Postfix) with ESMTPS id D3E346E120 for ; Tue, 22 Jun 2021 19:10:08 +0000 (UTC) Received: from mail01.mxhotel.dk (mail01.mxhotel.dk [91.221.196.236]) by mx2.smtp.larsendata.com (Halon) with ESMTPS id 7dd50cf3-d38d-11eb-a36f-0050568cd888; Tue, 22 Jun 2021 19:10:22 +0000 (UTC) Received: from ravnborg.org (80-162-45-141-cable.dk.customer.tdc.net [80.162.45.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: sam@ravnborg.org) by mail01.mxhotel.dk (Postfix) with ESMTPSA id 53825194B4A; Tue, 22 Jun 2021 21:10:11 +0200 (CEST) Date: Tue, 22 Jun 2021 21:10:03 +0200 X-Report-Abuse-To: abuse@mxhotel.dk From: Sam Ravnborg To: Daniel Vetter Subject: Re: [PATCH 07/15] drm/atomic-helper: make drm_gem_plane_helper_prepare_fb the default Message-ID: References: <20210622165511.3169559-1-daniel.vetter@ffwll.ch> <20210622165511.3169559-8-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210622165511.3169559-8-daniel.vetter@ffwll.ch> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , Daniel Vetter , Intel Graphics Development , Thomas Zimmermann , DRI Development Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Daniel, On Tue, Jun 22, 2021 at 06:55:03PM +0200, Daniel Vetter wrote: > There's a bunch of atomic drivers who don't do this quite correctly, > luckily most of them aren't in wide use or people would have noticed > the tearing. > > By making this the default we avoid the constant audit pain and can > additionally remove a ton of lines from vfuncs for a bit more clarity > in smaller drivers. > > While at it complain if there's a cleanup_fb hook but no prepare_fb > hook, because that makes no sense. I haven't found any driver which > violates this, but better safe than sorry. > > Subsequent patches will reap the benefits. > > Signed-off-by: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic_helper.c | 10 ++++++++++ > drivers/gpu/drm/drm_gem_atomic_helper.c | 3 +++ > include/drm/drm_modeset_helper_vtables.h | 7 +++++-- > 3 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 531f2374b072..9f6c5f21c4d6 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -2408,6 +2409,15 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev, > ret = funcs->prepare_fb(plane, new_plane_state); > if (ret) > goto fail; > + } else { > + WARN_ON_ONCE(funcs->cleanup_fb); > + > + if (!drm_core_check_feature(dev, DRIVER_GEM)) > + continue; > + > + ret = drm_gem_plane_helper_prepare_fb(plane, new_plane_state); > + if (ret) > + goto fail; > } > } > > diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c > index a27135084ae5..bc9396f2a0ed 100644 > --- a/drivers/gpu/drm/drm_gem_atomic_helper.c > +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c > @@ -135,6 +135,9 @@ > * GEM based framebuffer drivers which have their buffers always pinned in > * memory. > * > + * This function is the default implementation for GEM drivers of > + * &drm_plane_helper_funcs.prepare_fb if no callback is provided. > + * > * See drm_atomic_set_fence_for_plane() for a discussion of implicit and > * explicit fencing in atomic modeset updates. > */ > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index f3a4b47b3986..4e727261dca5 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -1178,8 +1178,11 @@ struct drm_plane_helper_funcs { > * equivalent functionality should be implemented through private > * members in the plane structure. > * > - * Drivers which always have their buffers pinned should use > - * drm_gem_plane_helper_prepare_fb() for this hook. > + * For GEM drivers who neither have a @prepare_fb not @cleanup_fb hook s/not/nor/ ?? > + * set drm_gem_plane_helper_prepare_fb() is called automatically to ^add comma? > + * implement this. Leave cleanup_fb out of the description to make it more readable. In the description of cleanup_fb you can document that it is wrong to have it without a matcching prepare_fb if you feel for it. Sam * Other drivers which need additional plane processing > + * can call drm_gem_plane_helper_prepare_fb() from their @prepare_fb > + * hook. > * > * The helpers will call @cleanup_fb with matching arguments for every > * successful call to this hook. > -- > 2.32.0.rc2 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DFCA4C2B9F4 for ; Tue, 22 Jun 2021 19:10:14 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A85BA601FE for ; Tue, 22 Jun 2021 19:10:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A85BA601FE Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B03386E877; Tue, 22 Jun 2021 19:10:10 +0000 (UTC) Received: from mx2.smtp.larsendata.com (mx2.smtp.larsendata.com [91.221.196.228]) by gabe.freedesktop.org (Postfix) with ESMTPS id D386D6E072 for ; Tue, 22 Jun 2021 19:10:08 +0000 (UTC) Received: from mail01.mxhotel.dk (mail01.mxhotel.dk [91.221.196.236]) by mx2.smtp.larsendata.com (Halon) with ESMTPS id 7dd50cf3-d38d-11eb-a36f-0050568cd888; Tue, 22 Jun 2021 19:10:22 +0000 (UTC) Received: from ravnborg.org (80-162-45-141-cable.dk.customer.tdc.net [80.162.45.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: sam@ravnborg.org) by mail01.mxhotel.dk (Postfix) with ESMTPSA id 53825194B4A; Tue, 22 Jun 2021 21:10:11 +0200 (CEST) Date: Tue, 22 Jun 2021 21:10:03 +0200 X-Report-Abuse-To: abuse@mxhotel.dk From: Sam Ravnborg To: Daniel Vetter Message-ID: References: <20210622165511.3169559-1-daniel.vetter@ffwll.ch> <20210622165511.3169559-8-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210622165511.3169559-8-daniel.vetter@ffwll.ch> Subject: Re: [Intel-gfx] [PATCH 07/15] drm/atomic-helper: make drm_gem_plane_helper_prepare_fb the default X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , Daniel Vetter , Intel Graphics Development , Thomas Zimmermann , DRI Development Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Hi Daniel, On Tue, Jun 22, 2021 at 06:55:03PM +0200, Daniel Vetter wrote: > There's a bunch of atomic drivers who don't do this quite correctly, > luckily most of them aren't in wide use or people would have noticed > the tearing. > > By making this the default we avoid the constant audit pain and can > additionally remove a ton of lines from vfuncs for a bit more clarity > in smaller drivers. > > While at it complain if there's a cleanup_fb hook but no prepare_fb > hook, because that makes no sense. I haven't found any driver which > violates this, but better safe than sorry. > > Subsequent patches will reap the benefits. > > Signed-off-by: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic_helper.c | 10 ++++++++++ > drivers/gpu/drm/drm_gem_atomic_helper.c | 3 +++ > include/drm/drm_modeset_helper_vtables.h | 7 +++++-- > 3 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 531f2374b072..9f6c5f21c4d6 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -2408,6 +2409,15 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev, > ret = funcs->prepare_fb(plane, new_plane_state); > if (ret) > goto fail; > + } else { > + WARN_ON_ONCE(funcs->cleanup_fb); > + > + if (!drm_core_check_feature(dev, DRIVER_GEM)) > + continue; > + > + ret = drm_gem_plane_helper_prepare_fb(plane, new_plane_state); > + if (ret) > + goto fail; > } > } > > diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c > index a27135084ae5..bc9396f2a0ed 100644 > --- a/drivers/gpu/drm/drm_gem_atomic_helper.c > +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c > @@ -135,6 +135,9 @@ > * GEM based framebuffer drivers which have their buffers always pinned in > * memory. > * > + * This function is the default implementation for GEM drivers of > + * &drm_plane_helper_funcs.prepare_fb if no callback is provided. > + * > * See drm_atomic_set_fence_for_plane() for a discussion of implicit and > * explicit fencing in atomic modeset updates. > */ > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index f3a4b47b3986..4e727261dca5 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -1178,8 +1178,11 @@ struct drm_plane_helper_funcs { > * equivalent functionality should be implemented through private > * members in the plane structure. > * > - * Drivers which always have their buffers pinned should use > - * drm_gem_plane_helper_prepare_fb() for this hook. > + * For GEM drivers who neither have a @prepare_fb not @cleanup_fb hook s/not/nor/ ?? > + * set drm_gem_plane_helper_prepare_fb() is called automatically to ^add comma? > + * implement this. Leave cleanup_fb out of the description to make it more readable. In the description of cleanup_fb you can document that it is wrong to have it without a matcching prepare_fb if you feel for it. Sam * Other drivers which need additional plane processing > + * can call drm_gem_plane_helper_prepare_fb() from their @prepare_fb > + * hook. > * > * The helpers will call @cleanup_fb with matching arguments for every > * successful call to this hook. > -- > 2.32.0.rc2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx