From: seanpaul@chromium.org (Sean Paul) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 4/7] drm/i2c: tda998x: convert to bridge driver Date: Fri, 10 Aug 2018 12:11:05 -0400 [thread overview] Message-ID: <20180810161105.GT20303@art_vandelay> (raw) In-Reply-To: <20180808221547.GN30658@n2100.armlinux.org.uk> On Wed, Aug 08, 2018 at 11:15:47PM +0100, Russell King - ARM Linux wrote: > On Wed, Aug 08, 2018 at 03:09:30PM -0400, Sean Paul wrote: > > > -static const struct drm_encoder_helper_funcs tda998x_encoder_helper_funcs = { > > > - .dpms = tda998x_encoder_dpms, > > > - .prepare = tda998x_encoder_prepare, > > > - .commit = tda998x_encoder_commit, > > > - .mode_set = tda998x_encoder_mode_set, > > > -}; > > > > Now that encoder is a stub, it should really be removed from here. The encoder > > should be instantiated elsewhere and attach the bridge to itself. There are a > > bunch of examples of this in bridge/ > > That's not possible at present - this driver has to remain compatible > with Armada and TI LCDC, both of which expect this driver to create > the encoder. Hmm, yeah, I just read that thread. I suppose once bridges play nice with components the encoder can migrate into the drivers? > > In any case, bridges are buggy with unbinding/rebinding as I've pointed > out several times in the past, but TDA998x used with Armada and TI LCDC > as it currently stands are not. So, to do this as a full conversion to > bridge and pushing the encoders into the DRM drivers results in a > regression for these two DRM drivers. I'm not willing to accept such > a regression, sorry. > > Thanks for the other cleanup suggestions, they can be done with a > later patch (these changes have already been been merged.) I still think you should get review from a bridge maintainer before it goes to drm-next. Sean > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up > According to speedtest.net: 13Mbps down 490kbps up -- Sean Paul, Software Engineer, Google / Chromium OS
WARNING: multiple messages have this Message-ID (diff)
From: Sean Paul <seanpaul@chromium.org> To: Russell King - ARM Linux <linux@armlinux.org.uk> Cc: David Airlie <airlied@linux.ie>, Liviu Dudau <Liviu.Dudau@arm.com>, dri-devel@lists.freedesktop.org, Tomi Valkeinen <tomi.valkeinen@ti.com>, Jyri Sarha <jsarha@ti.com>, Peter Rosin <peda@axentia.se>, linux-arm-kernel@lists.infradead.org, Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Subject: Re: [PATCH v2 4/7] drm/i2c: tda998x: convert to bridge driver Date: Fri, 10 Aug 2018 12:11:05 -0400 [thread overview] Message-ID: <20180810161105.GT20303@art_vandelay> (raw) In-Reply-To: <20180808221547.GN30658@n2100.armlinux.org.uk> On Wed, Aug 08, 2018 at 11:15:47PM +0100, Russell King - ARM Linux wrote: > On Wed, Aug 08, 2018 at 03:09:30PM -0400, Sean Paul wrote: > > > -static const struct drm_encoder_helper_funcs tda998x_encoder_helper_funcs = { > > > - .dpms = tda998x_encoder_dpms, > > > - .prepare = tda998x_encoder_prepare, > > > - .commit = tda998x_encoder_commit, > > > - .mode_set = tda998x_encoder_mode_set, > > > -}; > > > > Now that encoder is a stub, it should really be removed from here. The encoder > > should be instantiated elsewhere and attach the bridge to itself. There are a > > bunch of examples of this in bridge/ > > That's not possible at present - this driver has to remain compatible > with Armada and TI LCDC, both of which expect this driver to create > the encoder. Hmm, yeah, I just read that thread. I suppose once bridges play nice with components the encoder can migrate into the drivers? > > In any case, bridges are buggy with unbinding/rebinding as I've pointed > out several times in the past, but TDA998x used with Armada and TI LCDC > as it currently stands are not. So, to do this as a full conversion to > bridge and pushing the encoders into the DRM drivers results in a > regression for these two DRM drivers. I'm not willing to accept such > a regression, sorry. > > Thanks for the other cleanup suggestions, they can be done with a > later patch (these changes have already been been merged.) I still think you should get review from a bridge maintainer before it goes to drm-next. Sean > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up > According to speedtest.net: 13Mbps down 490kbps up -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2018-08-10 16:11 UTC|newest] Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-30 16:41 [PATCH v2 0/7] tda998x: allow use with bridge based devices Russell King - ARM Linux 2018-07-30 16:41 ` Russell King - ARM Linux 2018-07-30 16:42 ` [PATCH v2 1/7] drm/i2c: tda998x: find the drm_device via the drm_connector Russell King 2018-07-30 16:42 ` Russell King 2018-07-30 16:42 ` [PATCH v2 2/7] drm/i2c: tda998x: split tda998x_encoder_dpms into enable/disable Russell King 2018-07-30 16:42 ` Russell King 2018-07-31 5:46 ` Peter Rosin 2018-07-31 5:46 ` Peter Rosin 2018-07-30 16:42 ` [PATCH v2 3/7] drm/i2c: tda998x: move tda998x_set_config() into tda998x_create() Russell King 2018-07-30 16:42 ` Russell King 2018-07-30 16:42 ` [PATCH v2 4/7] drm/i2c: tda998x: convert to bridge driver Russell King 2018-07-30 16:42 ` Russell King 2018-07-31 7:37 ` Peter Rosin 2018-07-31 7:37 ` Peter Rosin 2018-08-08 19:09 ` Sean Paul 2018-08-08 19:09 ` Sean Paul 2018-08-08 22:15 ` Russell King - ARM Linux 2018-08-08 22:15 ` Russell King - ARM Linux 2018-08-10 16:11 ` Sean Paul [this message] 2018-08-10 16:11 ` Sean Paul 2018-08-10 16:50 ` Russell King - ARM Linux 2018-08-10 16:50 ` Russell King - ARM Linux 2018-08-10 17:02 ` Sean Paul 2018-08-10 17:02 ` Sean Paul 2018-08-10 17:16 ` Russell King - ARM Linux 2018-08-10 17:16 ` Russell King - ARM Linux 2018-08-14 10:42 ` Daniel Vetter 2018-08-14 10:42 ` Daniel Vetter 2018-08-14 10:48 ` Russell King - ARM Linux 2018-08-14 10:48 ` Russell King - ARM Linux 2018-08-14 11:11 ` Daniel Vetter 2018-08-14 11:11 ` Daniel Vetter 2018-08-27 16:15 ` Andrzej Hajda 2018-08-27 16:15 ` Andrzej Hajda 2018-08-27 17:59 ` Russell King - ARM Linux 2018-08-27 17:59 ` Russell King - ARM Linux 2018-08-28 7:31 ` Andrzej Hajda 2018-08-28 7:31 ` Andrzej Hajda 2018-07-30 16:42 ` [PATCH v2 5/7] drm/i2c: tda998x: allocate tda998x_priv inside tda998x_create() Russell King 2018-07-30 16:42 ` Russell King 2018-07-30 16:42 ` [PATCH v2 6/7] drm/i2c: tda998x: cleanup from previous changes Russell King 2018-07-30 16:42 ` Russell King 2018-07-30 16:42 ` [PATCH v2 7/7] drm/i2c: tda998x: register bridge outside of component helper Russell King 2018-07-30 16:42 ` Russell King 2018-08-27 16:19 ` Andrzej Hajda 2018-08-27 16:19 ` Andrzej Hajda 2018-07-31 5:44 ` [PATCH v2 0/7] tda998x: allow use with bridge based devices Peter Rosin 2018-07-31 5:44 ` Peter Rosin 2018-07-31 7:41 ` Russell King - ARM Linux 2018-07-31 7:41 ` Russell King - ARM Linux 2018-07-31 7:53 ` Peter Rosin 2018-07-31 7:53 ` Peter Rosin 2018-07-31 9:23 ` Russell King - ARM Linux 2018-07-31 9:23 ` Russell King - ARM Linux 2018-07-31 9:26 ` [PATCH 1/4] drm/i2c: tda998x: move mode_valid() to bridge Russell King 2018-07-31 9:26 ` Russell King 2018-08-27 16:24 ` Andrzej Hajda 2018-08-27 16:24 ` Andrzej Hajda 2018-07-31 9:26 ` [PATCH 2/4] drm/i2c: tda998x: get rid of private fill_modes function Russell King 2018-07-31 9:26 ` Russell King 2018-07-31 9:26 ` [PATCH 3/4] drm/i2c: tda998x: correct PLL divider calculation Russell King 2018-07-31 9:26 ` Russell King 2018-07-31 9:26 ` [PATCH 4/4] drm/i2c: tda998x: add support for pixel repeated modes Russell King 2018-07-31 9:26 ` Russell King 2018-07-31 9:42 ` Russell King - ARM Linux 2018-07-31 9:42 ` Russell King - ARM Linux 2018-07-31 10:43 ` [PATCH v2 0/7] tda998x: allow use with bridge based devices Peter Rosin 2018-07-31 10:43 ` Peter Rosin 2018-07-31 11:15 ` Russell King - ARM Linux 2018-07-31 11:15 ` Russell King - ARM Linux 2018-08-01 9:01 ` Peter Rosin 2018-08-01 9:01 ` Peter Rosin 2018-08-01 9:35 ` Russell King - ARM Linux 2018-08-01 9:35 ` Russell King - ARM Linux 2018-08-02 6:06 ` Peter Rosin 2018-08-02 6:06 ` Peter Rosin 2018-11-12 16:50 ` Peter Rosin 2018-11-12 16:50 ` Peter Rosin 2018-11-12 17:00 ` Russell King - ARM Linux 2018-11-12 17:00 ` Russell King - ARM Linux
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=20180810161105.GT20303@art_vandelay \ --to=seanpaul@chromium.org \ --cc=linux-arm-kernel@lists.infradead.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.