dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Brian Starkey <brian.starkey@arm.com>
Cc: Daniel Vetter <daniel@ffwll.ch>,
	Sean Paul <seanpaul@chromium.org>,
	Liviu Dudau <Liviu.Dudau@arm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i2c: tda998x: don't register the connector
Date: Thu, 22 Sep 2016 15:24:29 +0100	[thread overview]
Message-ID: <20160922142429.GX1041@n2100.armlinux.org.uk> (raw)
In-Reply-To: <20160922133842.GB5632@e106950-lin.cambridge.arm.com>

On Thu, Sep 22, 2016 at 02:38:45PM +0100, Brian Starkey wrote:
> However, without patching all three drivers in the same commit, there
> would always be some breakage. HDLCD and Mali-DP call
> drm_dev_register() before binding the components - this was needed to
> work with tda998x, which needed the device to be already registered
> before its bind callback runs.

I guess that is because drm_connector_register() relies on
drm_minor_register() having been previously run, which is rather
unfortunate.  That brings up the question about whether eliminating
the load callback is really such a wise move.

tda998x works fine either with or without the drm_connector_register()
call if it's bound within drm_dev_register(), which happens after the
minors have been registered.

> It's more proper to call drm_dev_register() as the very last thing
> (i.e. after component_bind_all()) to avoid races with userspace - but
> I couldn't do that without this change in tda998x first.

Actually, what you're saying is that you need to change tda998x in
lock-step with your drivers - if you have only the tda998x patch
applied, then you break.  If you apply the patch for your drivers,
you break.

That's really not good, and can be worked around by providing the
legacy ->load method, which just does the component_bind_all() call.
That makes you independent of the tda998x change, just like other
DRM drivers such as armada and tilcdc.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

  reply	other threads:[~2016-09-22 14:24 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-25 10:55 [PATCH] drm/i2c: tda998x: don't register the connector Brian Starkey
2016-07-25 12:25 ` Russell King - ARM Linux
2016-07-25 12:54   ` Brian Starkey
2016-07-25 15:08     ` Daniel Vetter
2016-08-08 16:04       ` Brian Starkey
2016-08-09  6:07         ` Daniel Vetter
2016-08-09 22:48           ` Russell King - ARM Linux
2016-09-21  8:57             ` Brian Starkey
2016-09-21 16:28               ` Russell King - ARM Linux
2016-09-22 10:39                 ` Brian Starkey
2016-09-22 10:51                   ` Russell King - ARM Linux
2016-09-22 11:22                     ` Sean Paul
2016-09-22 11:28                       ` Daniel Vetter
2016-09-22 13:38                         ` Brian Starkey
2016-09-22 14:24                           ` Russell King - ARM Linux [this message]
2016-09-22 12:09                       ` Russell King - ARM Linux
2016-09-22 12:32                         ` Sean Paul
2016-09-22 12:40                           ` Russell King - ARM Linux
2016-09-23  7:07                             ` Daniel Vetter
2016-09-22 14:14                       ` Brian Starkey
2016-09-23  7:05                         ` Daniel Vetter
2016-09-23  7:18                           ` [PATCH] Revert "drm/i2c: tda998x: don't register the connector" Sean Paul
2016-09-23  9:32                             ` liviu.dudau
2016-09-23 11:05                               ` Sean Paul
2016-09-23  9:34                           ` [PATCH] drm/i2c: tda998x: don't register the connector Liviu Dudau
2016-09-23 10:58                             ` Daniel Vetter
2016-09-23 12:22                               ` Lucas Stach
2016-09-23 12:52                               ` Brian Starkey
2016-09-23 13:13                                 ` Daniel Vetter
2016-09-23 14:05                                   ` Russell King - ARM Linux
2016-09-25 20:34                                     ` Daniel Vetter
2016-09-23 14:42                                   ` Brian Starkey
2016-09-25 20:38                                     ` Daniel Vetter

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=20160922142429.GX1041@n2100.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Liviu.Dudau@arm.com \
    --cc=brian.starkey@arm.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seanpaul@chromium.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).