From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751705AbcFCHgW (ORCPT ); Fri, 3 Jun 2016 03:36:22 -0400 Received: from down.free-electrons.com ([37.187.137.238]:44331 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750923AbcFCHgT (ORCPT ); Fri, 3 Jun 2016 03:36:19 -0400 Date: Fri, 3 Jun 2016 09:36:15 +0200 From: Boris Brezillon To: Laurent Pinchart Cc: David Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org, Daniel Vetter , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Kukjin Kim , Krzysztof Kozlowski , linux-samsung-soc@vger.kernel.org, intel-gfx@lists.freedesktop.org, Jani Nikula , Alexey Brodkin , Inki Dae , Joonyoung Shim , Seung-Woo Kim , Kyungmin Park , Stefan Agner , Alison Wang , Matthias Brugger , Rob Clark , Mark Yao , Heiko Stuebner , Benjamin Gaignard , Vincent Abriou , Maxime Ripard , Chen-Yu Tsai , Thierry Re ding , Stephen Warren , Alexandre Courbot , Eric Anholt , Gerd Hoffmann , linux-mediatek@lists.infradead.org, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-tegra@vger.kernel.org, virtualization@lists.linux-foundation.org Subject: Re: [PATCH 01/20] drm/atomic: Fix remaining places where !funcs->best_encoder is valid Message-ID: <20160603093615.14ba5fb8@bbrezillon> In-Reply-To: <8671722.LHaiaBNqfE@avalon> References: <1464877907-27723-1-git-send-email-boris.brezillon@free-electrons.com> <1464877907-27723-2-git-send-email-boris.brezillon@free-electrons.com> <8671722.LHaiaBNqfE@avalon> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 03 Jun 2016 00:05:46 +0300 Laurent Pinchart wrote: > Hi Boris, > > Thank you for the patch. > > On Thursday 02 Jun 2016 16:31:28 Boris Brezillon wrote: > > Adapt drm_pick_crtcs() and update_connector_routing() to fallback to > > drm_atomic_helper_best_encoder() if funcs->best_encoder() is NULL so > > that DRM drivers can leave this hook unassigned if they know they want > > to use drm_atomic_helper_best_encoder(). > > Could you please update include/drm/drm_modeset_helper_vtables.h to document > this new behaviour ? Sure. > > The only drawback I see in this patch is the additional object lookup > performed by drm_atomic_helper_best_encoder() at runtime. I wonder if we could > somehow cache the information, as the assignment can't change when there's a > 1:1 correspondence between encoders and connectors. > > > Signed-off-by: Boris Brezillon > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 4 +++- > > drivers/gpu/drm/drm_fb_helper.c | 13 ++++++++++++- > > 2 files changed, 15 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c index f6a3350..849d029 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -300,8 +300,10 @@ update_connector_routing(struct drm_atomic_state > > *state, if (funcs->atomic_best_encoder) > > new_encoder = funcs->atomic_best_encoder(connector, > > connector_state); > > - else > > + else if (funcs->best_encoder) > > new_encoder = funcs->best_encoder(connector); > > + else > > + new_encoder = drm_atomic_helper_best_encoder(connector); > > > > if (!new_encoder) { > > DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n", > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > > b/drivers/gpu/drm/drm_fb_helper.c index 7c2eb75..d44389a 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -2000,7 +2000,18 @@ static int drm_pick_crtcs(struct drm_fb_helper > > *fb_helper, my_score++; > > > > connector_funcs = connector->helper_private; > > - encoder = connector_funcs->best_encoder(connector); > > + > > + /* > > + * If the DRM device implements atomic hooks and ->best_encoder() is > > + * NULL we fallback to the default drm_atomic_helper_best_encoder() > > + * helper. > > + */ > > + if (fb_helper->dev->mode_config.funcs->atomic_commit && > > + !connector_funcs->best_encoder) > > + encoder = drm_atomic_helper_best_encoder(connector); > > + else > > + encoder = connector_funcs->best_encoder(connector); > > + > > if (!encoder) > > goto out; > -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com