AMD-GFX Archive on lore.kernel.org
 help / color / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Lyude Paul <lyude@redhat.com>
Cc: "Neil Armstrong" <narmstrong@baylibre.com>,
	"David Airlie" <airlied@linux.ie>,
	nouveau@lists.freedesktop.org, "Guido Günther" <agx@sigxcpu.org>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Oleg Vasilev" <oleg.vasilev@intel.com>,
	dri-devel@lists.freedesktop.org,
	"Meenakshikumar Somasundaram"
	<meenakshikumar.somasundaram@amd.com>,
	"Andrzej Hajda" <a.hajda@samsung.com>,
	"Aurabindo Pillai" <aurabindo.pillai@amd.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	Matt@freedesktop.org, "Tomi Valkeinen" <tomi.valkeinen@ti.com>,
	"Parshuram Thombare" <pthombar@cadence.com>,
	"Rodrigo Siqueira" <Rodrigo.Siqueira@amd.com>,
	"Michal Simek" <michal.simek@xilinx.com>,
	amd-gfx@lists.freedesktop.org,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	"Luben Tuikov" <luben.tuikov@amd.com>,
	"Ben Skeggs" <bskeggs@redhat.com>,
	"Swapnil Jakhade" <sjakhade@cadence.com>,
	"Thierry Reding" <treding@nvidia.com>,
	"Harry Wentland" <harry.wentland@amd.com>,
	"Imre Deak" <imre.deak@intel.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Jonas Karlman" <jonas@kwiboo.se>, "Leo Li" <sunpeng.li@amd.com>,
	intel-gfx@lists.freedesktop.org,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joe Perches" <joe@perches.com>,
	"Yuti Amonkar" <yamonkar@cadence.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Mikita Lipski" <mikita.lipski@amd.com>,
	"Sean Paul" <sean@poorly.run>,
	"Navid Emamdoost" <navid.emamdoost@gmail.com>,
	"Jernej Skrabec" <jernej.skrabec@siol.net>,
	"Chris Park" <Chris.Park@amd.com>,
	"Eryk Brol" <eryk.brol@amd.com>,
	"Hyun Kwon" <hyun.kwon@xilinx.com>,
	"Robert Foss" <robert.foss@linaro.org>,
	"Julia Lawall" <Julia.Lawall@inria.fr>,
	"Rob Clark" <robdclark@gmail.com>,
	"Boris Brezillon" <boris.brezillon@collabora.com>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH v3 03/20] drm/dp: Move i2c init to drm_dp_aux_init, add __must_check and fini
Date: Tue, 20 Apr 2021 02:16:13 +0300
Message-ID: <YH4PPbY1qqF2NtrN@intel.com> (raw)
In-Reply-To: <20210419225523.184856-4-lyude@redhat.com>

On Mon, Apr 19, 2021 at 06:55:05PM -0400, Lyude Paul wrote:
> When moving around drm_dp_aux_register() calls, it turned out we
> accidentally managed to cause issues with the Tegra driver due to the fact
> the Tegra driver would attempt to retrieve a reference to the AUX channel's
> i2c adapter - which wouldn't be initialized until drm_dp_aux_register() is
> called.
> 
> This doesn't actually make a whole ton of sense, as it's not unexpected for
> a driver to need to be able to use an AUX adapter before it's been
> registered. Likewise-it's not unexpected for a driver to try using the i2c
> adapter for said AUX channel before it's been registered as well. In fact,
> the current documentation for drm_dp_aux_init() even seems to imply that
> drm_dp_aux_init() is supposed to be handling i2c adapter creation for this
> precise reason - not drm_dp_aux_register().
> 
> Since the i2c adapter doesn't need to be linked to the DRM device in any
> way, we can just fix this problem by moving i2c adapter creation out of
> drm_dp_aux_register() and into drm_dp_aux_init(). Additionally, since this
> means that drm_dp_aux_init() can fail we go ahead and add a __must_check
> attribute to it so that drivers don't ignore its return status on failures.
> And finally, we add a drm_dp_aux_fini() and hook it up in all DRM drivers
> across the kernel to take care of cleaning up the i2c adapter once it's no
> longer needed.
> 
> This should also fix the regressions noted in the Tegra driver.

The init vs. register split is intentional. Registering the thing
and allowing userspace access to it before the rest of the driver
is ready isn't particularly great. For a while now we've tried to
move towards an architecture where the driver is fully initialzied
before anything gets exposed to userspace.

-- 
Ville Syrjälä
Intel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  reply index

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-19 22:55 [PATCH v3 00/20] drm: Use new DRM printk funcs (like drm_dbg_*()) in DP helpers Lyude Paul
2021-04-19 22:55 ` [PATCH v3 01/20] drm/amdgpu: Add error handling to amdgpu_dm_initialize_dp_connector() Lyude Paul
2021-04-21 14:23   ` Mikita Lipski
2021-04-21 15:22     ` Deucher, Alexander
2021-04-19 22:55 ` [PATCH v3 02/20] drm/dp: Add __no_check to drm_dp_aux_register() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 03/20] drm/dp: Move i2c init to drm_dp_aux_init, add __must_check and fini Lyude Paul
2021-04-19 23:16   ` Ville Syrjälä [this message]
2021-04-22 17:18     ` Lyude Paul
2021-04-22 22:33       ` Lyude Paul
2021-04-23  4:11         ` Lyude Paul
2021-04-23 12:39           ` Thierry Reding
2021-04-23 17:53             ` Lyude Paul
2021-04-23 12:38         ` Thierry Reding
2021-04-23 12:34       ` Thierry Reding
2021-04-19 22:55 ` [PATCH v3 04/20] drm/bridge/cdns-mhdp8546: Register DP aux channel with userspace Lyude Paul
2021-04-19 22:55 ` [PATCH v3 05/20] drm/nouveau/kms/nv50-: Move AUX adapter reg to connector late register/early unregister Lyude Paul
2021-04-19 22:55 ` [PATCH v3 06/20] drm/dp: Add backpointer to drm_device in drm_dp_aux Lyude Paul
2021-04-19 22:55 ` [PATCH v3 07/20] drm/dp: Clarify DP AUX registration time Lyude Paul
2021-04-19 22:55 ` [PATCH v3 08/20] drm/dp: Pass drm_dp_aux to drm_dp_link_train_clock_recovery_delay() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 09/20] drm/dp: Pass drm_dp_aux to drm_dp*_link_train_channel_eq_delay() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 10/20] drm/dp: Always print aux channel name in logs Lyude Paul
2021-04-19 22:55 ` [PATCH v3 11/20] drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_detect() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 12/20] drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_set_tmds_output() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 13/20] drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_max_tmds_clock() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 14/20] drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_get_tmds_output() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 15/20] drm/dp_dual_mode: Pass drm_device to drm_lspcon_(get|set)_mode() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 16/20] drm/dp_mst: Pass drm_dp_mst_topology_mgr to drm_dp_get_vc_payload_bw() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 17/20] drm/print: Handle potentially NULL drm_devices in drm_dbg_* Lyude Paul
2021-04-19 22:55 ` [PATCH v3 18/20] drm/dp: Convert drm_dp_helper.c to using drm_err/drm_dbg_*() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 19/20] drm/dp_dual_mode: Convert drm_dp_dual_mode_helper.c to using drm_err/drm_dbg_kms() Lyude Paul
2021-04-19 22:55 ` [PATCH v3 20/20] drm/dp_mst: Convert drm_dp_mst_topology.c to drm_err()/drm_dbg*() Lyude Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YH4PPbY1qqF2NtrN@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=Chris.Park@amd.com \
    --cc=Julia.Lawall@inria.fr \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=Matt@freedesktop.org \
    --cc=Rodrigo.Siqueira@amd.com \
    --cc=a.hajda@samsung.com \
    --cc=agx@sigxcpu.org \
    --cc=airlied@linux.ie \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=aurabindo.pillai@amd.com \
    --cc=boris.brezillon@collabora.com \
    --cc=bskeggs@redhat.com \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eryk.brol@amd.com \
    --cc=harry.wentland@amd.com \
    --cc=hyun.kwon@xilinx.com \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=jernej.skrabec@siol.net \
    --cc=joe@perches.com \
    --cc=jonas@kwiboo.se \
    --cc=jonathanh@nvidia.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=luben.tuikov@amd.com \
    --cc=lyude@redhat.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=meenakshikumar.somasundaram@amd.com \
    --cc=michal.simek@xilinx.com \
    --cc=mikita.lipski@amd.com \
    --cc=mripard@kernel.org \
    --cc=narmstrong@baylibre.com \
    --cc=navid.emamdoost@gmail.com \
    --cc=nouveau@lists.freedesktop.org \
    --cc=oleg.vasilev@intel.com \
    --cc=pthombar@cadence.com \
    --cc=robdclark@gmail.com \
    --cc=robert.foss@linaro.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=sam@ravnborg.org \
    --cc=sean@poorly.run \
    --cc=sjakhade@cadence.com \
    --cc=sunpeng.li@amd.com \
    --cc=thierry.reding@gmail.com \
    --cc=tomi.valkeinen@ti.com \
    --cc=treding@nvidia.com \
    --cc=tzimmermann@suse.de \
    --cc=yamonkar@cadence.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

AMD-GFX Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/amd-gfx/0 amd-gfx/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 amd-gfx amd-gfx/ https://lore.kernel.org/amd-gfx \
		amd-gfx@lists.freedesktop.org
	public-inbox-index amd-gfx

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.amd-gfx


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git