From: linux@arm.linux.org.uk (Russell King - ARM Linux) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 5/5] DRM: Armada: add support for drm tda19988 driver Date: Mon, 7 Oct 2013 10:44:04 +0100 [thread overview] Message-ID: <20131007094404.GI12758@n2100.arm.linux.org.uk> (raw) In-Reply-To: <20131007111807.5e86ea6e@armhf> On Mon, Oct 07, 2013 at 11:18:07AM +0200, Jean-Francois Moine wrote: > On Sun, 06 Oct 2013 23:11:56 +0100 > Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > --- > > drivers/gpu/drm/armada/Kconfig | 9 +++++++ > > drivers/gpu/drm/armada/armada_drv.c | 42 +++++++++++++++++++++++++++++++++++ > > 2 files changed, 51 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/gpu/drm/armada/Kconfig b/drivers/gpu/drm/armada/Kconfig > > index c7a0a94..87e62dd 100644 > > --- a/drivers/gpu/drm/armada/Kconfig > > +++ b/drivers/gpu/drm/armada/Kconfig > > @@ -13,3 +13,12 @@ config DRM_ARMADA > > This driver provides no built-in acceleration; acceleration is > > performed by other IP found on the SoC. This driver provides > > kernel mode setting and buffer management to userspace. > > + > > +config DRM_ARMADA_TDA1998X > > + bool "Support TDA1998X HDMI output" > > + depends on DRM_ARMADA != n > > + depends on I2C && DRM_I2C_NXP_TDA998X = y > > + default y > > + help > > + Support the TDA1998x HDMI output device found on the Solid-Run > > + CuBox. > > It seems we are going backwards: as the Armada based boards will soon > move to full DT (mvebu), you are making an exception for the Cubox, so > that there should be Cubox specific kernels. I don't like that... *Ignored*. You know why. > > diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c > > index db62f1b..69517cf 100644 > > --- a/drivers/gpu/drm/armada/armada_drv.c > > +++ b/drivers/gpu/drm/armada/armada_drv.c > > @@ -16,6 +16,42 @@ > > #include <drm/armada_drm.h> > > #include "armada_ioctlP.h" > > > > +#ifdef CONFIG_DRM_ARMADA_TDA1998X > > +#include <drm/i2c/tda998x.h> > > +#include "armada_slave.h" > > + > > +static struct tda998x_encoder_params params = { > > + /* With 0x24, there is no translation between vp_out and int_vp > > + FB LCD out Pins VIP Int Vp > > + R:23:16 R:7:0 VPC7:0 7:0 7:0[R] > > + G:15:8 G:15:8 VPB7:0 23:16 23:16[G] > > + B:7:0 B:23:16 VPA7:0 15:8 15:8[B] > > + */ > > + .swap_a = 2, > > + .swap_b = 3, > > + .swap_c = 4, > > + .swap_d = 5, > > + .swap_e = 0, > > + .swap_f = 1, > > I still don't agree. You don't need to invert R <-> B for the Cubox at > the tda998x level: this may be done setting as it should be the > CFG_GRA_SWAPRB flag of the lcd register LCD_SPU_DMA_CTRL0. You are totally and utterly wrong there. We need R and B presented on their correct lanes to the TDA998x so that the Armadas YUV->RGB conversion works. Setting CFG_GRA_SWAPRB does not swap the YUV output to match, neither does setting any of the other bits. CFG_GRA_SWAPRB is all about the _graphics_ _framebuffer_ format, it's got nothing to do at all with how the output is wired. > > + .audio_cfg = BIT(2), > > + .audio_frame[1] = 1, > > + .audio_format = AFMT_SPDIF, > > + .audio_sample_rate = 44100, > > These values are rather mysterious! Also I'm going to ignore this comment, because quite honestly, I think this is worthless. You haven't investigated how the TDA998x actually gets setup by Rabeeh.
WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux <linux@arm.linux.org.uk> To: Jean-Francois Moine <moinejf@free.fr> Cc: Jason Cooper <jason@lakedaemon.net>, David Airlie <airlied@linux.ie>, dri-devel@lists.freedesktop.org, Rob Clark <robdclark@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> Subject: Re: [PATCH 5/5] DRM: Armada: add support for drm tda19988 driver Date: Mon, 7 Oct 2013 10:44:04 +0100 [thread overview] Message-ID: <20131007094404.GI12758@n2100.arm.linux.org.uk> (raw) In-Reply-To: <20131007111807.5e86ea6e@armhf> On Mon, Oct 07, 2013 at 11:18:07AM +0200, Jean-Francois Moine wrote: > On Sun, 06 Oct 2013 23:11:56 +0100 > Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > --- > > drivers/gpu/drm/armada/Kconfig | 9 +++++++ > > drivers/gpu/drm/armada/armada_drv.c | 42 +++++++++++++++++++++++++++++++++++ > > 2 files changed, 51 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/gpu/drm/armada/Kconfig b/drivers/gpu/drm/armada/Kconfig > > index c7a0a94..87e62dd 100644 > > --- a/drivers/gpu/drm/armada/Kconfig > > +++ b/drivers/gpu/drm/armada/Kconfig > > @@ -13,3 +13,12 @@ config DRM_ARMADA > > This driver provides no built-in acceleration; acceleration is > > performed by other IP found on the SoC. This driver provides > > kernel mode setting and buffer management to userspace. > > + > > +config DRM_ARMADA_TDA1998X > > + bool "Support TDA1998X HDMI output" > > + depends on DRM_ARMADA != n > > + depends on I2C && DRM_I2C_NXP_TDA998X = y > > + default y > > + help > > + Support the TDA1998x HDMI output device found on the Solid-Run > > + CuBox. > > It seems we are going backwards: as the Armada based boards will soon > move to full DT (mvebu), you are making an exception for the Cubox, so > that there should be Cubox specific kernels. I don't like that... *Ignored*. You know why. > > diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c > > index db62f1b..69517cf 100644 > > --- a/drivers/gpu/drm/armada/armada_drv.c > > +++ b/drivers/gpu/drm/armada/armada_drv.c > > @@ -16,6 +16,42 @@ > > #include <drm/armada_drm.h> > > #include "armada_ioctlP.h" > > > > +#ifdef CONFIG_DRM_ARMADA_TDA1998X > > +#include <drm/i2c/tda998x.h> > > +#include "armada_slave.h" > > + > > +static struct tda998x_encoder_params params = { > > + /* With 0x24, there is no translation between vp_out and int_vp > > + FB LCD out Pins VIP Int Vp > > + R:23:16 R:7:0 VPC7:0 7:0 7:0[R] > > + G:15:8 G:15:8 VPB7:0 23:16 23:16[G] > > + B:7:0 B:23:16 VPA7:0 15:8 15:8[B] > > + */ > > + .swap_a = 2, > > + .swap_b = 3, > > + .swap_c = 4, > > + .swap_d = 5, > > + .swap_e = 0, > > + .swap_f = 1, > > I still don't agree. You don't need to invert R <-> B for the Cubox at > the tda998x level: this may be done setting as it should be the > CFG_GRA_SWAPRB flag of the lcd register LCD_SPU_DMA_CTRL0. You are totally and utterly wrong there. We need R and B presented on their correct lanes to the TDA998x so that the Armadas YUV->RGB conversion works. Setting CFG_GRA_SWAPRB does not swap the YUV output to match, neither does setting any of the other bits. CFG_GRA_SWAPRB is all about the _graphics_ _framebuffer_ format, it's got nothing to do at all with how the output is wired. > > + .audio_cfg = BIT(2), > > + .audio_frame[1] = 1, > > + .audio_format = AFMT_SPDIF, > > + .audio_sample_rate = 44100, > > These values are rather mysterious! Also I'm going to ignore this comment, because quite honestly, I think this is worthless. You haven't investigated how the TDA998x actually gets setup by Rabeeh.
next prev parent reply other threads:[~2013-10-07 9:44 UTC|newest] Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-10-06 22:07 [PATCH 0/5] Armada DRM stuff Russell King - ARM Linux 2013-10-06 22:07 ` Russell King - ARM Linux 2013-10-06 22:07 ` [PATCH 1/5] drm/i2c: tda998x: set VIF for full range, underscanned display Russell King 2013-10-06 22:07 ` Russell King 2013-10-07 8:59 ` Jean-Francois Moine 2013-10-07 8:59 ` Jean-Francois Moine 2013-10-18 15:00 ` Rob Clark 2013-10-18 15:00 ` Rob Clark 2013-10-06 22:08 ` [PATCH 2/5] DRM: Armada: Add Armada DRM driver Russell King 2013-10-06 22:08 ` Russell King 2013-10-10 21:25 ` Rob Clark 2013-10-10 21:25 ` Rob Clark 2013-10-10 21:59 ` Russell King - ARM Linux 2013-10-10 21:59 ` Russell King - ARM Linux 2013-10-10 22:23 ` Rob Clark 2013-10-10 22:23 ` Rob Clark 2013-10-10 22:53 ` Russell King - ARM Linux 2013-10-10 22:53 ` Russell King - ARM Linux 2013-10-11 0:10 ` Rob Clark 2013-10-11 0:10 ` Rob Clark 2013-10-06 22:09 ` [PATCH 3/5] DRM: Armada: Add support for ARGB 32x64 or 64x32 hardware cursors Russell King 2013-10-06 22:09 ` Russell King 2013-10-07 9:01 ` Jean-Francois Moine 2013-10-07 9:01 ` Jean-Francois Moine 2013-10-07 9:40 ` Russell King - ARM Linux 2013-10-07 9:40 ` Russell King - ARM Linux 2013-10-07 10:09 ` Jean-Francois Moine 2013-10-07 10:09 ` Jean-Francois Moine 2013-10-07 10:32 ` Russell King - ARM Linux 2013-10-07 10:32 ` Russell King - ARM Linux 2013-10-07 12:29 ` Siarhei Siamashka 2013-10-07 12:29 ` Siarhei Siamashka 2013-10-07 12:50 ` Russell King - ARM Linux 2013-10-07 12:50 ` Russell King - ARM Linux 2013-10-17 23:58 ` Rob Clark 2013-10-17 23:58 ` Rob Clark 2013-10-18 14:31 ` Alex Deucher 2013-10-18 14:31 ` Alex Deucher 2013-10-06 22:10 ` [PATCH 4/5] DRM: Armada: start of MMP2/MMP3 support Russell King 2013-10-06 22:10 ` Russell King 2013-10-18 0:11 ` Rob Clark 2013-10-18 0:11 ` Rob Clark 2013-10-06 22:11 ` [PATCH 5/5] DRM: Armada: add support for drm tda19988 driver Russell King 2013-10-06 22:11 ` Russell King 2013-10-07 9:18 ` Jean-Francois Moine 2013-10-07 9:18 ` Jean-Francois Moine 2013-10-07 9:44 ` Russell King - ARM Linux [this message] 2013-10-07 9:44 ` Russell King - ARM Linux 2013-10-07 10:48 ` Jean-Francois Moine 2013-10-07 10:48 ` Jean-Francois Moine 2013-10-07 11:09 ` Russell King - ARM Linux 2013-10-07 11:09 ` Russell King - ARM Linux 2013-10-07 11:29 ` Sebastian Hesselbarth 2013-10-07 11:29 ` Sebastian Hesselbarth 2013-10-07 15:53 ` Mark Brown 2013-10-07 15:53 ` Mark Brown 2013-10-07 16:08 ` Sebastian Hesselbarth 2013-10-07 16:08 ` Sebastian Hesselbarth 2013-10-07 17:05 ` Mark Brown 2013-10-07 17:05 ` Mark Brown 2013-10-07 12:03 ` Jean-Francois Moine 2013-10-07 12:03 ` Jean-Francois Moine 2013-10-07 12:36 ` Russell King - ARM Linux 2013-10-07 12:36 ` Russell King - ARM Linux 2013-10-07 14:59 ` Rob Clark 2013-10-07 14:59 ` Rob Clark 2013-10-08 9:19 ` Jean-Francois Moine 2013-10-08 9:19 ` Jean-Francois Moine 2013-10-08 9:49 ` Russell King - ARM Linux 2013-10-08 9:49 ` Russell King - ARM Linux 2013-10-08 15:34 ` Jean-Francois Moine 2013-10-08 15:34 ` Jean-Francois Moine 2013-10-18 0:20 ` Rob Clark 2013-10-18 0:20 ` Rob Clark 2013-10-08 12:07 ` Rob Clark 2013-10-08 12:07 ` Rob Clark 2013-10-07 21:47 ` [PATCH 0/5] Armada DRM stuff Sebastian Hesselbarth 2013-10-07 21:47 ` Sebastian Hesselbarth 2013-10-09 14:31 ` Russell King - ARM Linux 2013-10-09 14:31 ` Russell King - ARM Linux 2013-10-09 14:48 ` Rob Clark 2013-10-09 14:48 ` Rob Clark 2013-10-18 15:15 ` [GIT PULL] Armada DRM support Russell King - ARM Linux 2013-10-18 15:15 ` Russell King - ARM Linux 2013-10-22 13:36 ` Russell King - ARM Linux 2013-10-22 13:36 ` 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=20131007094404.GI12758@n2100.arm.linux.org.uk \ --to=linux@arm.linux.org.uk \ --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.