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 6B8F0C433F5 for ; Fri, 15 Oct 2021 15:19:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5387460E0C for ; Fri, 15 Oct 2021 15:19:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240911AbhJOPVK convert rfc822-to-8bit (ORCPT ); Fri, 15 Oct 2021 11:21:10 -0400 Received: from mga17.intel.com ([192.55.52.151]:15887 "EHLO mga17.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232267AbhJOPVK (ORCPT ); Fri, 15 Oct 2021 11:21:10 -0400 X-IronPort-AV: E=McAfee;i="6200,9189,10138"; a="208731858" X-IronPort-AV: E=Sophos;i="5.85,376,1624345200"; d="scan'208";a="208731858" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Oct 2021 08:18:48 -0700 X-IronPort-AV: E=Sophos;i="5.85,376,1624345200"; d="scan'208";a="481724815" Received: from tzahur-mobl.ger.corp.intel.com (HELO localhost) ([10.251.211.201]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Oct 2021 08:18:37 -0700 From: Jani Nikula To: Ville =?utf-8?B?U3lyasOkbMOk?= Cc: Claudio Suarez , 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 , Joonas Lahtinen , Rodrigo Vivi , Alex Deucher , Christian =?utf-8?Q?K=C3=B6nig?= , 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 , Benjamin Gaignard , Sandy Huang , heiko@sntech.de, Andrzej Hajda , Neil Armstrong , Robert Foss , Ben Skeggs , nouveau@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20211015113713.630119-1-cssk@net-c.es> <20211015113713.630119-16-cssk@net-c.es> <87a6jav4n3.fsf@intel.com> Date: Fri, 15 Oct 2021 18:18:34 +0300 Message-ID: <874k9iuxit.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org On Fri, 15 Oct 2021, Ville Syrjälä wrote: > On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote: >> On Fri, 15 Oct 2021, Claudio Suarez wrote: >> > Once EDID is parsed, the monitor HDMI support information is available >> > through drm_display_info.is_hdmi. Retriving the same information with >> > drm_detect_hdmi_monitor() is less efficient. Change to >> > drm_display_info.is_hdmi where possible. >> > >> > This is a TODO task in Documentation/gpu/todo.rst >> > >> > Signed-off-by: Claudio Suarez >> > --- >> > drivers/gpu/drm/i915/display/intel_connector.c | 5 +++++ >> > drivers/gpu/drm/i915/display/intel_connector.h | 1 + >> > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- >> > drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++- >> > 4 files changed, 9 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c >> > index 9bed1ccecea0..3346b55df6e1 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_connector.c >> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c >> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector, >> > return ret; >> > } >> > >> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector) >> > +{ >> > + return connector->display_info.is_hdmi; >> > +} >> > + >> >> A helper like this belongs in drm, not i915. Seems useful in other >> drivers too. > > Not sure it's actually helpful for i915. We end up having to root around > in the display_info in a lot of places anyway. So a helper for single > boolean seems a bit out of place perhaps. *shrug* Maybe it's just my frustration at the lack of interfaces and poking around in the depths of nested structs and pointer chasing that's coming through. You just need to change so many things if you want to later refactor where "is hdmi" comes from and is stored. Anyway, if a helper is being added like in this series, I think it should be one helper in drm, not redundant copies in multiple drivers. Or we should not have the helper(s) at all. One or the other, not the worst of both worlds. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center 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 C4514C433F5 for ; Fri, 15 Oct 2021 15:19:13 +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 872A661181 for ; Fri, 15 Oct 2021 15:19:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 872A661181 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7FA2C6EDE4; Fri, 15 Oct 2021 15:19:05 +0000 (UTC) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id DE2D56EDE4; Fri, 15 Oct 2021 15:19:03 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10138"; a="208731855" X-IronPort-AV: E=Sophos;i="5.85,376,1624345200"; d="scan'208";a="208731855" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Oct 2021 08:18:48 -0700 X-IronPort-AV: E=Sophos;i="5.85,376,1624345200"; d="scan'208";a="481724815" Received: from tzahur-mobl.ger.corp.intel.com (HELO localhost) ([10.251.211.201]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Oct 2021 08:18:37 -0700 From: Jani Nikula To: Ville =?utf-8?B?U3lyasOkbMOk?= Cc: Claudio Suarez , 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 , Joonas Lahtinen , Rodrigo Vivi , Alex Deucher , Christian =?utf-8?Q?K=C3=B6nig?= , 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 , Benjamin Gaignard , Sandy Huang , heiko@sntech.de, Andrzej Hajda , Neil Armstrong , Robert Foss , Ben Skeggs , nouveau@lists.freedesktop.org In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20211015113713.630119-1-cssk@net-c.es> <20211015113713.630119-16-cssk@net-c.es> <87a6jav4n3.fsf@intel.com> Date: Fri, 15 Oct 2021 18:18:34 +0300 Message-ID: <874k9iuxit.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Nouveau] [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi X-BeenThere: nouveau@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Nouveau development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" On Fri, 15 Oct 2021, Ville Syrj=C3=A4l=C3=A4 wrote: > On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote: >> On Fri, 15 Oct 2021, Claudio Suarez wrote: >> > Once EDID is parsed, the monitor HDMI support information is available >> > through drm_display_info.is_hdmi. Retriving the same information with >> > drm_detect_hdmi_monitor() is less efficient. Change to >> > drm_display_info.is_hdmi where possible. >> > >> > This is a TODO task in Documentation/gpu/todo.rst >> > >> > Signed-off-by: Claudio Suarez >> > --- >> > drivers/gpu/drm/i915/display/intel_connector.c | 5 +++++ >> > drivers/gpu/drm/i915/display/intel_connector.h | 1 + >> > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- >> > drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++- >> > 4 files changed, 9 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/= gpu/drm/i915/display/intel_connector.c >> > index 9bed1ccecea0..3346b55df6e1 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_connector.c >> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c >> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *con= nector, >> > return ret; >> > } >> >=20=20 >> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector) >> > +{ >> > + return connector->display_info.is_hdmi; >> > +} >> > + >>=20 >> A helper like this belongs in drm, not i915. Seems useful in other >> drivers too. > > Not sure it's actually helpful for i915. We end up having to root around > in the display_info in a lot of places anyway. So a helper for single > boolean seems a bit out of place perhaps. *shrug* Maybe it's just my frustration at the lack of interfaces and poking around in the depths of nested structs and pointer chasing that's coming through. You just need to change so many things if you want to later refactor where "is hdmi" comes from and is stored. Anyway, if a helper is being added like in this series, I think it should be one helper in drm, not redundant copies in multiple drivers. Or we should not have the helper(s) at all. One or the other, not the worst of both worlds. BR, Jani. --=20 Jani Nikula, Intel Open Source Graphics Center