Re: [Nouveau] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Claudio Suarez <cssk@net-c.es>
Cc: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
	linux-tegra@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	"David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Pan Xinhui" <Xinhui.Pan@amd.com>,
	"Emma Anholt" <emma@anholt.net>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Patrik Jakobsson" <patrik.r.jakobsson@gmail.com>,
	"Jingoo Han" <jingoohan1@gmail.com>,
	"Rob Clark" <robdclark@gmail.com>, "Sean Paul" <sean@poorly.run>,
	linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
	"Chen-Yu Tsai" <wens@csie.org>,
	"Sandy Huang" <hjc@rock-chips.com>,
	heiko@sntech.de, "Neil Armstrong" <narmstrong@baylibre.com>,
	"Robert Foss" <robert.foss@linaro.org>,
	"Ben Skeggs" <bskeggs@redhat.com>,
	nouveau@lists.freedesktop.org
Subject: Re: [Nouveau] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Date: Fri, 22 Oct 2021 15:01:52 +0300	[thread overview]
Message-ID: <YXKoMEF/gU98cL9n@intel.com> (raw)
In-Reply-To: <YXKRnUHWuboQKBF1@zorro.micasa>

On Fri, Oct 22, 2021 at 12:25:33PM +0200, Claudio Suarez wrote:
> On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
> > On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
> > > drm_get_edid() internally calls to drm_connector_update_edid_property()
> > > and then drm_add_display_info(), which parses the EDID.
> > > This happens in the function intel_hdmi_set_edid() and
> > > intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
> > > 
> > > 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
> > 
> > I meant we need to examine all call chains that can lead to
> > .detect() to make sure all of them do in fact update the
> > display_info beforehand.
> 
> Well, I studied it carefully and, yes, all call chains that can lead to
> drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info
> beforehand. In the case that this doesn't happen, the code is unchanged.
> 
> Do you want I explain the changes in the code here again ? Or do you want
> to me change the commit message to be more clear ? In the first case, I can
> write here a detailed explanation. In the second case I can make a longer commit
> message.
> 
> Or both?

I want all those call chains explained in the commit message,
otherwise I have no easy way to confirm whether the change
is correct or not.

-- 
Ville Syrjälä
Intel