All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Philippe CORNU <philippe.cornu@st.com>
Cc: Archit Taneja <architt@codeaurora.org>,
	Sean Paul <seanpaul@chromium.org>,
	Nickey Yang <nickey.yang@rock-chips.com>,
	"mark.yao@rock-chips.com" <mark.yao@rock-chips.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"heiko@sntech.de" <heiko@sntech.de>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"airlied@linux.ie" <airlied@linux.ie>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-rockchip@lists.infradead.org"
	<linux-rockchip@lists.infradead.org>,
	"hl@rock-chips.com" <hl@rock-chips.com>,
	"zyw@rock-chips.com" <zyw@rock-chips.com>,
	"xbl@rock-chips.com" <xbl@rock-chips.com>,
	Kristian Kristensen <hoegsberg@gmail.com>
Subject: Re: [PATCH v3 3/6] drm/rockchip/dsi: correct Feedback divider setting
Date: Thu, 26 Oct 2017 14:32:05 -0700	[thread overview]
Message-ID: <20171026213204.GA55212@google.com> (raw)
In-Reply-To: <39862e7a-b03f-f5d9-54db-ff9aef294a9c@st.com>

(correction zyw's email ".comg" is not a TLD!)

Hi,

On Thu, Oct 26, 2017 at 09:44:14AM +0000, Philippe CORNU wrote:
> On 10/26/2017 06:13 AM, Archit Taneja wrote:
> > On 10/26/2017 06:39 AM, Brian Norris wrote:
> >> On Wed, Oct 25, 2017 at 03:57:19AM -0400, Sean Paul wrote:
> >>> Archit asked a question about moving to
> >>> dw-mipi-dsi
> >>
> >> That question made me think though: this approach seems backwards. It
> >> seems like someone did copy/paste/fork, and then we're asking the
> >> authors of the original driver to un-fork? It seems like this should
> >> happen the other way around -- those trying to support a new incarnation
> >> should have looked to try to abstract the original driver for their
> >> uses first.
> > 
> > Yes, ST wanted to replicate rockchip's version of the mipi DSI driver and
> > put it in their folder. If they did that, their KMS driver would have been
> > the third driver to implement a third instance of the DW DSI controller 
> > driver.
> > Hisilicon and Rockchip being the other 2.

I hadn't noticed Hisilicon's version. That's an unfortunate start :(

> > It was either that or attempt at a common DSI DW bridge driver. I suggested
> > the latter.
> > 
> > The ST guys have abstracted out the PHY pieces, which they knew varied 
> > between
> > rockchip and ST. Ideally, they should have also tried to create a RFC 
> > patch to
> > make the rockchip driver use the bridge too. But they didn't do that, and
> > the rockchip or hisilicon people were interested in even looking at it,
> > even after I CC'ed them.

I see. At least the current code from Philippe isn't that big, and it is
indeed fairly similar.

But I still think the way to get cooperation upstream is to enforce it;
so there was a 3rd option to the above -- don't merge *any* new driver
without posting at least an attempt to unify the existing drivers.

> >> IIUC, that's exactly what Rockchip did for much of their Analogix eDP
> >> code -- they reworked the Exynos DP driver to split common Analogix code
> >> from any Exynos-specific bits.
> > 
> > I get that. I had hoped either ST or Rockchip guys would have done the 
> > similar
> > thing, but no one volunteered.

:(

Nickey, can you confirm that you or someone on your team will look at
utilizing the common driver? Please reply to this email thread before
sending another version of this series.

> >> And actually, the current stuff in
> >> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c is completely unused. It
> >> exports some functions, but I see no users of it. Is that intended? Is
> > 
> > The ST kms driver uses it:
> > 
> > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > 
> 
> I confirm STM32 chipsets use the Synopsys dw dsi bridge driver.
> 
> I plan to improve this bridge driver by adding new features (see todos + 
> dsi read, command mode with bta & gpio...).
> 
> For the first commit, I did my best to keep the source code as close as 
> possible to the Rockchip version, in order to ease the port for Rockchip 
> guys.

That's nice to see, even if it still isn't ideal.

> >> somebody already working on refactoring existing Rockchip code to use
> >> this?
> > 
> > I don't know. If rockchip isn't interested in doing it, we can check with
> > Philippe from ST if he can try creating a RFC that converts the rockchip
> > driver to use the dw-mipi-dsi driver.
> 
> I am not really interested in doing this port for Rockchip (or Hisilicon 
> or i.MX...) but happy to help anyone that wants to use the dw-mipi-dsi 
> bridge driver :)

Well, see my comments above. Your "interest" is obviously in merging
code to support your own IP, but the community can *make* it your
interest by requiring you do the unification before your code goes
upstream. Apparently that's not the policy here though.

Brian

  reply	other threads:[~2017-10-26 21:32 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-25  3:50 [PATCH v3 1/6] drm/rockchip/dsi: Define and use macros for PHY register addresses Nickey Yang
2017-10-25  3:50 ` Nickey Yang
2017-10-25  3:50 ` [PATCH v3 2/6] drm/rockchip/dsi: correct phy parameter setting Nickey Yang
2017-10-25  3:50   ` Nickey Yang
2017-10-25  7:49   ` Sean Paul
2017-10-25  3:51 ` [PATCH v3 3/6] drm/rockchip/dsi: correct Feedback divider setting Nickey Yang
2017-10-25  3:51   ` Nickey Yang
2017-10-25  7:57   ` Sean Paul
2017-10-25  7:57     ` Sean Paul
2017-10-26  1:09     ` Brian Norris
2017-10-26  1:09       ` Brian Norris
2017-10-26  4:13       ` Archit Taneja
2017-10-26  4:13         ` Archit Taneja
     [not found]         ` <c1f17f1e-6e4f-d44d-348e-43157474f3c9-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-10-26  9:44           ` Philippe CORNU
2017-10-26  9:44             ` Philippe CORNU
2017-10-26 21:32             ` Brian Norris [this message]
2017-11-28  0:29             ` Brian Norris
2017-11-28  0:34               ` Brian Norris
2017-11-28  0:34                 ` Brian Norris
2017-10-25  3:51 ` [PATCH v3 4/6] drm/rockchip/dsi: add dual mipi channel support Nickey Yang
2017-10-25  8:04   ` Sean Paul
2017-10-25  8:04     ` Sean Paul
2017-10-26  5:11     ` Archit Taneja
2017-10-26  5:11       ` Archit Taneja
2017-10-25  3:51 ` [PATCH v3 5/6] dt-bindings: add the rockchip,dual-channel for dw-mipi-dsi Nickey Yang
2017-10-26  4:53   ` [PATCH v3 5/6] dt-bindings: add the rockchip, dual-channel " Archit Taneja
2017-11-30 17:32     ` Nickey Yang
2017-11-30 17:32       ` Nickey Yang
2017-12-01 12:59       ` Archit Taneja
2017-12-01 12:59         ` Archit Taneja
2017-12-05  1:19         ` Brian Norris
2017-12-05  1:19           ` Brian Norris
2017-12-05  5:16           ` Archit Taneja
2017-12-05  5:16             ` Archit Taneja
2017-10-25  3:51 ` [PATCH v3 6/6] arm64: dts: rockchip: add mipi_dsi1 support for rk3399 Nickey Yang
2017-10-25  7:47 ` [PATCH v3 1/6] drm/rockchip/dsi: Define and use macros for PHY register addresses Sean Paul
2017-10-25  7:47   ` Sean 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=20171026213204.GA55212@google.com \
    --to=briannorris@chromium.org \
    --cc=airlied@linux.ie \
    --cc=architt@codeaurora.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=heiko@sntech.de \
    --cc=hl@rock-chips.com \
    --cc=hoegsberg@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=mark.yao@rock-chips.com \
    --cc=nickey.yang@rock-chips.com \
    --cc=philippe.cornu@st.com \
    --cc=robh+dt@kernel.org \
    --cc=seanpaul@chromium.org \
    --cc=xbl@rock-chips.com \
    --cc=zyw@rock-chips.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
Be 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.