From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Stefan Agner <stefan@agner.ch>
Cc: linus.walleij@linaro.org, airlied@linux.ie, robh+dt@kernel.org,
mark.rutland@arm.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, p.zabel@pengutronix.de,
kernel@pengutronix.de, fabio.estevam@nxp.com, linux-imx@nxp.com,
architt@codeaurora.org, a.hajda@samsung.com, gustavo@padovan.org,
maarten.lankhorst@linux.intel.com, sean@poorly.run,
marcel.ziswiler@toradex.com, max.krummenacher@toradex.com,
dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings
Date: Fri, 14 Sep 2018 12:23:52 +0300 [thread overview]
Message-ID: <6714863.eJMplaj6yU@avalon> (raw)
In-Reply-To: <20180912183222.25414-2-stefan@agner.ch>
Hi Stefan,
Thankk you for the patch.
On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote:
> The DRM bus flags conveys additional information on pixel data on
> the bus. All currently available bus flags might be of interest for
> a bridge. In the case at hand a dumb VGA bridge needs a specific
> data enable polarity (DRM_BUS_FLAG_DE_LOW).
>
> Replace the sampling_edge field with input_bus_flags and allow all
> currently documented bus flags.
>
> This changes the perspective from sampling side to the driving
> side for the currently supported flags. We assume that the sampling
> edge is always the opposite of the driving edge (hence we need to
> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an
> assumption we already make for displays. For all we know it is a
> safe assumption for bridges too.
Please don't, that will get confusing very quickly. Flag names such as
DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's only a
small comment next to their definition, which will easily be overlooked. I'd
rather update the definition to cover both sampling and driving, and document
the input_bus_flags field to explain that all flags refer to sampling.
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 +++---
> include/drm/drm_bridge.h | 11 +++++------
> 2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2
> 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
> */
> static const struct drm_bridge_timings default_dac_timings = {
> /* Timing specifications, datasheet page 7 */
> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> .setup_time_ps = 500,
> .hold_time_ps = 1500,
> };
> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings
> default_dac_timings = { */
> static const struct drm_bridge_timings ti_ths8134_dac_timings = {
> /* From timing diagram, datasheet page 9 */
> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> /* From datasheet, page 12 */
> .setup_time_ps = 3000,
> /* I guess this means latched input */
> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings
> ti_ths8134_dac_timings = { */
> static const struct drm_bridge_timings ti_ths8135_dac_timings = {
> /* From timing diagram, datasheet page 14 */
> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> /* From datasheet, page 16 */
> .setup_time_ps = 2000,
> .hold_time_ps = 500,
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index bd850747ce54..45e90f4b46c3 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -244,14 +244,13 @@ struct drm_bridge_funcs {
> */
> struct drm_bridge_timings {
> /**
> - * @sampling_edge:
> + * @input_bus_flags:
> *
> - * Tells whether the bridge samples the digital input signal
> - * from the display engine on the positive or negative edge of the
> - * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE
> - * bitwise flags from the DRM connector (bit 2 and 3 valid).
> + * Additional settings this bridge requires for the pixel data on
> + * the input bus (e.g. pixel signal polarity). See also
> + * &drm_display_info->bus_flags.
> */
> - u32 sampling_edge;
> + u32 input_bus_flags;
> /**
> * @setup_time_ps:
> *
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2018-09-14 9:23 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-12 18:32 [PATCH v2 0/8] drm/bridge: add bus flag support Stefan Agner
2018-09-12 18:32 ` [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings Stefan Agner
2018-09-14 9:23 ` Laurent Pinchart [this message]
2018-09-18 18:14 ` Stefan Agner
2018-09-22 12:06 ` Laurent Pinchart
2018-09-12 18:32 ` [PATCH v2 2/8] drm/pl111: simplify bridge timing support Stefan Agner
2018-09-12 18:32 ` [PATCH v2 3/8] drm/bridge: simplify bridge timing info Stefan Agner
2018-09-14 9:34 ` Laurent Pinchart
2018-09-12 18:32 ` [PATCH v2 4/8] drm/bridge: allow to specify data-enable polarity Stefan Agner
2018-09-12 18:32 ` [PATCH v2 5/8] dt-bindings: display: add data-enable polarity property Stefan Agner
2018-09-26 21:01 ` Rob Herring
2018-09-12 18:32 ` [PATCH v2 6/8] drm/imx: support handling bridge timings bus flags Stefan Agner
2018-09-13 8:38 ` Philipp Zabel
2018-09-13 17:03 ` Stefan Agner
2018-09-14 10:00 ` Laurent Pinchart
2018-09-12 18:32 ` [PATCH v2 7/8] ARM: dts: imx6qdl-apalis: add VGA support Stefan Agner
2018-09-12 18:32 ` [PATCH v2 8/8] ARM: dts: imx6qdl-apalis: add GPIO I2C node for DDC Stefan Agner
2018-09-14 9:07 ` [PATCH v2 0/8] drm/bridge: add bus flag support Linus Walleij
2018-09-14 9:35 ` Laurent Pinchart
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=6714863.eJMplaj6yU@avalon \
--to=laurent.pinchart@ideasonboard.com \
--cc=a.hajda@samsung.com \
--cc=airlied@linux.ie \
--cc=architt@codeaurora.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=fabio.estevam@nxp.com \
--cc=gustavo@padovan.org \
--cc=kernel@pengutronix.de \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=marcel.ziswiler@toradex.com \
--cc=mark.rutland@arm.com \
--cc=max.krummenacher@toradex.com \
--cc=p.zabel@pengutronix.de \
--cc=robh+dt@kernel.org \
--cc=s.hauer@pengutronix.de \
--cc=sean@poorly.run \
--cc=shawnguo@kernel.org \
--cc=stefan@agner.ch \
/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).