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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A7CEC433F5 for ; Fri, 15 Oct 2021 19:24:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 823E261181 for ; Fri, 15 Oct 2021 19:24:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230372AbhJOT00 (ORCPT ); Fri, 15 Oct 2021 15:26:26 -0400 Received: from ip-16.mailobj.net ([213.182.54.16]:53378 "EHLO msg-6.mailo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242695AbhJOT0Z (ORCPT ); Fri, 15 Oct 2021 15:26:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=net-c.es; s=mailo; t=1634325849; bh=LHI6ADKUuEjRgd/13zbEVoP8yqLJTf3ZSPQveWR4Xeg=; h=X-EA-Auth:Date:From:To:Cc:Subject:Message-ID:References: MIME-Version:Content-Type:Content-Transfer-Encoding:In-Reply-To; b=UJkcX3Tm6RHULPirG9jbEdfFYGGJ66I9KBQKuzi98ISfle+pBczUkzMcwmIzRVLdT MAh0MVlf/AYndTRFTFK655yY2Mgf5bP1nrEWc/pBXWK9J27FJZxPow/BYvc+lLN2ir EtHptpAEuwYVBB73L+5SXGwDanjW0SDVYPlyj5SQ= Received: by b-3.in.mailobj.net [192.168.90.13] with ESMTP via ip-206.mailobj.net [213.182.55.206] Fri, 15 Oct 2021 21:24:09 +0200 (CEST) X-EA-Auth: U6CSUQYVFt3UVX/XYpcq7X1KrM+JbBDIgrc/Xwcsszd8MPNiA5z5prKhMOKAhZFdLHqNfioDKmq9YtwQVNP17+FoeAaZQdMb Date: Fri, 15 Oct 2021 21:24:06 +0200 From: Claudio Suarez To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-tegra@vger.kernel.org, intel-gfx@lists.freedesktop.org, David Airlie , Daniel Vetter , Laurent Pinchart , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Alex Deucher , Christian =?iso-8859-1?Q?K=F6nig?= , Pan Xinhui , Emma Anholt , Maxime Ripard , Thierry Reding , Patrik Jakobsson , Jingoo Han , Rob Clark , Sean Paul , linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, Chen-Yu Tsai , Sandy Huang , heiko@sntech.de, Neil Armstrong , Robert Foss , Ben Skeggs , nouveau@lists.freedesktop.org Subject: Re: [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info Message-ID: References: <20211015113713.630119-1-cssk@net-c.es> <20211015113713.630119-2-cssk@net-c.es> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org On Fri, Oct 15, 2021 at 03:03:13PM +0300, Ville Syrjälä wrote: > On Fri, Oct 15, 2021 at 01:36:59PM +0200, Claudio Suarez wrote: > > According to the documentation, drm_add_edid_modes > > "... Also fills out the &drm_display_info structure and ELD in @connector > > with any information which can be derived from the edid." > > > > drm_add_edid_modes accepts a struct edid *edid parameter which may have a > > value or may be null. When it is not null, connector->display_info and > > connector->eld are updated according to the edid. When edid=NULL, only > > connector->eld is reset. Reset connector->display_info to be consistent > > and accurate. > > > > Signed-off-by: Claudio Suarez > > --- > > drivers/gpu/drm/drm_edid.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 6325877c5fd6..6cbe09b2357c 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) > > > > if (edid == NULL) { > > clear_eld(connector); > > + drm_reset_display_info(connector); > > return 0; > > } > > if (!drm_edid_is_valid(edid)) { > > clear_eld(connector); > > + drm_reset_display_info(connector); > > Looks easier if you pull both of those out from these branches and > just call them unconditionally at the start. After looking at the full code, I am not sure. This is the code: ================== int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) { int num_modes = 0; u32 quirks; if (edid == NULL) { clear_eld(connector); drm_reset_display_info(connector); <--- added by me return 0; } if (!drm_edid_is_valid(edid)) { clear_eld(connector); drm_reset_display_info(connector); <--- added by me drm_warn(connector->dev, "%s: EDID invalid.\n", connector->name); return 0; } drm_edid_to_eld(connector, edid); quirks = drm_add_display_info(connector, edid); etc... ================= If we move those out of these branches and edid != NULL, we are executing an unnecessary clear_eld(connector) and an unnecessary drm_reset_display_info(connector) because the fields will be set in the next drm_edid_to_eld(connector, edid) and drm_add_display_info(connector, edid) Do we want this ? BR Claudio Suarez