Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
From: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
To: Andrzej Hajda <a.hajda@samsung.com>
Cc: Rob Clark <robdclark@gmail.com>, Mark Brown <broonie@kernel.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] regmap: Add DSI bus support
Date: Fri, 12 Jul 2019 08:22:33 -0600
Message-ID: <CAOCk7NoyCmPQF3s4GWD1Oa4t5hdqi6vdcOdHyJpo3Gc1JQqXcw@mail.gmail.com> (raw)
In-Reply-To: <10b1313f-7a60-df04-a9e3-76649b74f2f0@samsung.com>

On Fri, Jul 12, 2019 at 7:01 AM Andrzej Hajda <a.hajda@samsung.com> wrote:
>
> On 11.07.2019 15:56, Rob Clark wrote:
> > On Thu, Jul 11, 2019 at 6:11 AM Andrzej Hajda <a.hajda@samsung.com> wrote:
> >> On 06.07.2019 03:06, Mark Brown wrote:
> >>> On Wed, Jul 03, 2019 at 02:45:12PM -0700, Jeffrey Hugo wrote:
> >>>> Add basic support with a simple implementation that utilizes the generic
> >>>> read/write commands to allow device registers to be configured.
> >>> This looks good to me but I really don't know anything about DSI,
> >>> I'd appreciate some review from other people who do.  I take it
> >>> there's some spec thing in DSI that says registers and bytes must
> >>> both be 8 bit?
> >>
> >> I am little bit confused about regmap usage here. On the one hand it
> >> nicely fits to this specific driver, probably because it already uses
> >> regmap_i2c.
> >>
> >> On the other it will be unusable for almost all current DSI drivers and
> >> probably for most new drivers. Why?
> >>
> >> 1. DSI protocol defines actually more than 30 types of transactions[1],
> >> but this patchset implements only few of them (dsi generic write/read
> >> family). Is it possible to implement multiple types of transactions in
> >> regmap?
> >>
> >> 2. There is already some set of helpers which uses dsi bus, rewriting it
> >> on regmap is possible or driver could use of regmap and direct access
> >> together, the question is if it is really necessary.
> >>
> >> 3. DSI devices are no MFDs so regmap abstraction has no big value added
> >> (correct me, if there are other significant benefits).
> >>
> > I assume it is not *just* this one bridge that can be programmed over
> > either i2c or dsi, depending on how things are wired up on the board.
> > It certainly would be nice for regmap to support this case, so we
> > don't have to write two different bridge drivers for the same bridge.
> > I wouldn't expect a panel that is only programmed via dsi to use this.
>
>
> On the other side supporting DSI and I2C in one driver is simply matter
> of writing proper accesors.

To me, this reads like your counter argument to not using regmap, is
to reinvent regmap.  Maybe I don't understand what you are proposing
here, but it sounds like remove the regmap support, define sn65
specific accessors that just before sending the write to the bus does
a check if the access needs to go over i2c or DSI.  Feels like a
clunky version of regmap to me.  Why not use the existing "generic"
framework?

To your point that DSI defines over 30 message types, yes it does, but
that seems to be outside of the scope.  How many of those are actually
for doing register access?  I'm thinking just 4 (technically a hair
more than that because of the multiple version of the same message) -
generic read, generic write, dcs read, dcs write.  I don't view regmap
as a generic abstraction layer over a particular mechanism, and thus
needs to support everything that mechanism does.  Sending sync
commands, or pixel data over DSI is outside the scope of regmap to me.

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-03 21:43 [PATCH 0/2] ti-sn65dsi86 DSI configuration support Jeffrey Hugo
2019-07-03 21:45 ` [PATCH 1/2] regmap: Add DSI bus support Jeffrey Hugo
2019-07-06  1:06   ` Mark Brown
2019-07-10 18:08     ` Jeffrey Hugo
2019-07-11 14:41       ` Mark Brown
2019-07-11 13:11     ` Andrzej Hajda
2019-07-11 13:56       ` Rob Clark
2019-07-12 13:01         ` Andrzej Hajda
2019-07-12 14:22           ` Jeffrey Hugo [this message]
2019-07-13  0:49             ` Rob Clark
2019-07-11 14:50       ` Mark Brown
2019-07-15  8:38         ` Andrzej Hajda
2019-07-03 21:45 ` [PATCH 2/2] drm/bridge: ti-sn65dsi86: Add support to be a DSI device Jeffrey Hugo

Reply instructions:

You may reply publically 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=CAOCk7NoyCmPQF3s4GWD1Oa4t5hdqi6vdcOdHyJpo3Gc1JQqXcw@mail.gmail.com \
    --to=jeffrey.l.hugo@gmail.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=a.hajda@samsung.com \
    --cc=airlied@linux.ie \
    --cc=bjorn.andersson@linaro.org \
    --cc=broonie@kernel.org \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robdclark@gmail.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

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/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 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org linux-arm-msm@archiver.kernel.org
	public-inbox-index linux-arm-msm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


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