[PATCH 0/3] drm/omap: misc fixes

[PATCH 0/3] drm/omap: misc fixes

[Tomi Valkeinen]

Hi,

Here's three fixes for omapdrm:

- fix synclost flood on omap3
- fix analog tv-out videomode check
- fix DSI PLL setup

Tony, Aaro, Nikolaus, can you see if these fix the issues you're seeing?

 Tomi

Tomi Valkeinen (3):
  drm/omap: work-around for omap3 display enable
  drm/omap: fix analog tv-out modecheck
  drm/omap: fix i886 work-around

 drivers/gpu/drm/omapdrm/dss/dispc.c     | 14 +++++++
 drivers/gpu/drm/omapdrm/dss/dss.h       |  3 ++
 drivers/gpu/drm/omapdrm/dss/pll.c       | 29 ++++++++++-----
 drivers/gpu/drm/omapdrm/dss/venc.c      | 65 ++++++++++++++++++++++++++-------
 drivers/gpu/drm/omapdrm/dss/video-pll.c |  2 +
 5 files changed, 90 insertions(+), 23 deletions(-)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm/omap: work-around for omap3 display enable

[Tomi Valkeinen]

Seems that on omap3 enabling a crtc without any planes causes a sync
lost flood. This only happens on the first enable, and after that it
works. This looks like an HW issue.

It's unclear why this is happening or how to fix it, but as a quick
work-around, this patch enables i734 errata work-around for omap2 and
omap3 too. The errata work-around enables and disables the LCD output
with a plane once when waking up the DSS IP, and it seems to resolve the
omap3 problem too. It is unclear if omap2 has the same issue, but it
probably has and the WA should have no side effects so it should be safe
to enable on omap2 too.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 5ac0145fbae6..75e89707a70a 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -4004,6 +4004,11 @@ static const struct dispc_features omap24xx_dispc_feats = {
 	.no_framedone_tv	=	true,
 	.set_max_preload	=	false,
 	.last_pixel_inc_missing	=	true,
+	/*
+	 * HACK: see comment in omap34xx_rev1_0_dispc_feats. OMAP2 probably
+	 * has the same issue.
+	 */
+	.has_gamma_i734_bug	=	true,
 };
 
 static const struct dispc_features omap34xx_rev1_0_dispc_feats = {
@@ -4025,6 +4030,13 @@ static const struct dispc_features omap34xx_rev1_0_dispc_feats = {
 	.no_framedone_tv	=	true,
 	.set_max_preload	=	false,
 	.last_pixel_inc_missing	=	true,
+	/*
+	 * HACK: OMAP3 doesn't have i734, but enabling the lcd output without
+	 * planes causes synclost flood. This only happens on initial enable,
+	 * not after that.
+	 * Piggyback on i734 flag until we understand this better.
+	 */
+	.has_gamma_i734_bug	=	true,
 };
 
 static const struct dispc_features omap34xx_rev3_0_dispc_feats = {
@@ -4046,6 +4058,8 @@ static const struct dispc_features omap34xx_rev3_0_dispc_feats = {
 	.no_framedone_tv	=	true,
 	.set_max_preload	=	false,
 	.last_pixel_inc_missing	=	true,
+	/* HACK: see comment in omap34xx_rev1_0_dispc_feats */
+	.has_gamma_i734_bug	=	true,
 };
 
 static const struct dispc_features omap44xx_dispc_feats = {
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/omap: work-around for omap3 display enable

[Aaro Koskinen]

Hi,

On Tue, Jun 13, 2017 at 12:02:08PM +0300, Tomi Valkeinen wrote:
> Seems that on omap3 enabling a crtc without any planes causes a sync
> lost flood. This only happens on the first enable, and after that it
> works. This looks like an HW issue.
> 
> It's unclear why this is happening or how to fix it, but as a quick
> work-around, this patch enables i734 errata work-around for omap2 and
> omap3 too. The errata work-around enables and disables the LCD output
> with a plane once when waking up the DSS IP, and it seems to resolve the
> omap3 problem too. It is unclear if omap2 has the same issue, but it
> probably has and the WA should have no side effects so it should be safe
> to enable on omap2 too.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>

This fixes the LCD errors I'm seeing on N900 with v4.11, and I get a working
display.

A.

> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 5ac0145fbae6..75e89707a70a 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -4004,6 +4004,11 @@ static const struct dispc_features omap24xx_dispc_feats = {
>  	.no_framedone_tv	=	true,
>  	.set_max_preload	=	false,
>  	.last_pixel_inc_missing	=	true,
> +	/*
> +	 * HACK: see comment in omap34xx_rev1_0_dispc_feats. OMAP2 probably
> +	 * has the same issue.
> +	 */
> +	.has_gamma_i734_bug	=	true,
>  };
>  
>  static const struct dispc_features omap34xx_rev1_0_dispc_feats = {
> @@ -4025,6 +4030,13 @@ static const struct dispc_features omap34xx_rev1_0_dispc_feats = {
>  	.no_framedone_tv	=	true,
>  	.set_max_preload	=	false,
>  	.last_pixel_inc_missing	=	true,
> +	/*
> +	 * HACK: OMAP3 doesn't have i734, but enabling the lcd output without
> +	 * planes causes synclost flood. This only happens on initial enable,
> +	 * not after that.
> +	 * Piggyback on i734 flag until we understand this better.
> +	 */
> +	.has_gamma_i734_bug	=	true,
>  };
>  
>  static const struct dispc_features omap34xx_rev3_0_dispc_feats = {
> @@ -4046,6 +4058,8 @@ static const struct dispc_features omap34xx_rev3_0_dispc_feats = {
>  	.no_framedone_tv	=	true,
>  	.set_max_preload	=	false,
>  	.last_pixel_inc_missing	=	true,
> +	/* HACK: see comment in omap34xx_rev1_0_dispc_feats */
> +	.has_gamma_i734_bug	=	true,
>  };
>  
>  static const struct dispc_features omap44xx_dispc_feats = {
> -- 
> 2.7.4
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/omap: work-around for omap3 display enable

[Tomi Valkeinen]


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 13/06/17 12:02, Tomi Valkeinen wrote:
> Seems that on omap3 enabling a crtc without any planes causes a sync
> lost flood. This only happens on the first enable, and after that it
> works. This looks like an HW issue.
> 
> It's unclear why this is happening or how to fix it, but as a quick
> work-around, this patch enables i734 errata work-around for omap2 and
> omap3 too. The errata work-around enables and disables the LCD output
> with a plane once when waking up the DSS IP, and it seems to resolve the
> omap3 problem too. It is unclear if omap2 has the same issue, but it
> probably has and the WA should have no side effects so it should be safe
> to enable on omap2 too.

I was again debugging and testing this problem, and I don't think this patch
works very well. I'm getting endless sync losts again.

Here's another try, this time changing how the omapdrm commits the new setup.
At least for me this seems to work better.

 Tomi


From fc5cc9678e130196012c17b37e555d53d3d3476b Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date: Wed, 23 Aug 2017 12:19:02 +0300
Subject: [PATCHv2] drm/omap: work-around for omap3 display enable

Seems that on omap3 enabling a crtc without any planes causes a sync
lost flood. This only happens on the first enable, and after that it
works. This looks like an HW issue and it's unclear why this is
happening or how to fix it.

This started happening after 897145d0c7010b4e07fa9bc674b1dfb9a2c6fff9
("drm/omapdrm: Move commit_modeset_enables() before commit_planes()")
which, as a work-around, changed omapdrm first to do the modeset enable,
and plane set only after that. This WA should be fine on all DSS
versions, but apparently OMAP3 DSS is an exception.

This patch reverts that work-around for OMAP3 DSS.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 47 ++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 9b3c36b48356..cdf5b0601eba 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -84,23 +84,36 @@ static void omap_atomic_commit_tail(struct drm_atomic_state *old_state)
 	/* Apply the atomic update. */
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
 
-	/* With the current dss dispc implementation we have to enable
-	 * the new modeset before we can commit planes. The dispc ovl
-	 * configuration relies on the video mode configuration been
-	 * written into the HW when the ovl configuration is
-	 * calculated.
-	 *
-	 * This approach is not ideal because after a mode change the
-	 * plane update is executed only after the first vblank
-	 * interrupt. The dispc implementation should be fixed so that
-	 * it is able use uncommitted drm state information.
-	 */
-	drm_atomic_helper_commit_modeset_enables(dev, old_state);
-	omap_atomic_wait_for_completion(dev, old_state);
-
-	drm_atomic_helper_commit_planes(dev, old_state, 0);
-
-	drm_atomic_helper_commit_hw_done(old_state);
+	if (priv->omaprev != 0x3430) {
+		/* With the current dss dispc implementation we have to enable
+		 * the new modeset before we can commit planes. The dispc ovl
+		 * configuration relies on the video mode configuration been
+		 * written into the HW when the ovl configuration is
+		 * calculated.
+		 *
+		 * This approach is not ideal because after a mode change the
+		 * plane update is executed only after the first vblank
+		 * interrupt. The dispc implementation should be fixed so that
+		 * it is able use uncommitted drm state information.
+		 */
+		drm_atomic_helper_commit_modeset_enables(dev, old_state);
+		omap_atomic_wait_for_completion(dev, old_state);
+
+		drm_atomic_helper_commit_planes(dev, old_state, 0);
+
+		drm_atomic_helper_commit_hw_done(old_state);
+	} else {
+		/*
+		 * OMAP3 DSS seems to have issues with the work-around above,
+		 * resulting in endless sync losts if a crtc is enabled without
+		 * a plane. For now, skip the WA for OMAP3.
+		 */
+		drm_atomic_helper_commit_planes(dev, old_state, 0);
+
+		drm_atomic_helper_commit_modeset_enables(dev, old_state);
+
+		drm_atomic_helper_commit_hw_done(old_state);
+	}
 
 	/*
 	 * Wait for completion of the page flips to ensure that old buffers
-- 
2.7.4


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/omap: work-around for omap3 display enable

[Aaro Koskinen]

Hi,

On Wed, Aug 23, 2017 at 12:33:08PM +0300, Tomi Valkeinen wrote:
> On 13/06/17 12:02, Tomi Valkeinen wrote:
> > Seems that on omap3 enabling a crtc without any planes causes a sync
> > lost flood. This only happens on the first enable, and after that it
> > works. This looks like an HW issue.
> > 
> > It's unclear why this is happening or how to fix it, but as a quick
> > work-around, this patch enables i734 errata work-around for omap2 and
> > omap3 too. The errata work-around enables and disables the LCD output
> > with a plane once when waking up the DSS IP, and it seems to resolve the
> > omap3 problem too. It is unclear if omap2 has the same issue, but it
> > probably has and the WA should have no side effects so it should be safe
> > to enable on omap2 too.
> 
> I was again debugging and testing this problem, and I don't think this patch
> works very well. I'm getting endless sync losts again.
> 
> Here's another try, this time changing how the omapdrm commits the new setup.
> At least for me this seems to work better.
>
> From fc5cc9678e130196012c17b37e555d53d3d3476b Mon Sep 17 00:00:00 2001
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Date: Wed, 23 Aug 2017 12:19:02 +0300
> Subject: [PATCHv2] drm/omap: work-around for omap3 display enable

Thanks! I was finally able to test this (with v4.13). Multiple boots
and now the display is always working fine.

A.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/omap: fix analog tv-out modecheck

[Tomi Valkeinen]

omapdrm rejects all venc (analog tv-out) videomodes, due to somewhat
strict checking of the values, making tv-out unusable.

We only support two videomodes, one for PAL and one for NTSC, so instead
of trying to check every field in the videomode struct, this patch makes
the driver check only the pixel clock and the size of the display.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/venc.c | 65 ++++++++++++++++++++++++++++++--------
 1 file changed, 51 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c b/drivers/gpu/drm/omapdrm/dss/venc.c
index 19d14957f566..68e73277719e 100644
--- a/drivers/gpu/drm/omapdrm/dss/venc.c
+++ b/drivers/gpu/drm/omapdrm/dss/venc.c
@@ -263,6 +263,12 @@ static const struct venc_config venc_config_pal_bdghi = {
 	.fid_ext_start_y__fid_ext_offset_y	= 0x01380005,
 };
 
+enum venc_videomode {
+	VENC_MODE_UNKNOWN,
+	VENC_MODE_PAL,
+	VENC_MODE_NTSC,
+};
+
 const struct videomode omap_dss_pal_vm = {
 	.hactive	= 720,
 	.vactive	= 574,
@@ -299,6 +305,24 @@ const struct videomode omap_dss_ntsc_vm = {
 };
 EXPORT_SYMBOL(omap_dss_ntsc_vm);
 
+static enum venc_videomode venc_get_videomode(const struct videomode *vm)
+{
+	if (!(vm->flags & DISPLAY_FLAGS_INTERLACED))
+		return VENC_MODE_UNKNOWN;
+
+	if (vm->pixelclock == omap_dss_pal_vm.pixelclock &&
+	    vm->hactive == omap_dss_pal_vm.hactive &&
+	    vm->vactive == omap_dss_pal_vm.vactive)
+		return VENC_MODE_PAL;
+
+	if (vm->pixelclock == omap_dss_ntsc_vm.pixelclock &&
+	    vm->hactive == omap_dss_ntsc_vm.hactive &&
+	    vm->vactive == omap_dss_ntsc_vm.vactive)
+		return VENC_MODE_NTSC;
+
+	return VENC_MODE_UNKNOWN;
+}
+
 static struct {
 	struct platform_device *pdev;
 	void __iomem *base;
@@ -424,14 +448,14 @@ static void venc_runtime_put(void)
 
 static const struct venc_config *venc_timings_to_config(struct videomode *vm)
 {
-	if (memcmp(&omap_dss_pal_vm, vm, sizeof(*vm)) == 0)
+	switch (venc_get_videomode(vm)) {
+	default:
+		WARN_ON_ONCE(1);
+	case VENC_MODE_PAL:
 		return &venc_config_pal_trm;
-
-	if (memcmp(&omap_dss_ntsc_vm, vm, sizeof(*vm)) == 0)
+	case VENC_MODE_NTSC:
 		return &venc_config_ntsc_trm;
-
-	BUG();
-	return NULL;
+	}
 }
 
 static int venc_power_on(struct omap_dss_device *dssdev)
@@ -542,15 +566,28 @@ static void venc_display_disable(struct omap_dss_device *dssdev)
 static void venc_set_timings(struct omap_dss_device *dssdev,
 			     struct videomode *vm)
 {
+	struct videomode actual_vm;
+
 	DSSDBG("venc_set_timings\n");
 
 	mutex_lock(&venc.venc_lock);
 
+	switch (venc_get_videomode(vm)) {
+	default:
+		WARN_ON_ONCE(1);
+	case VENC_MODE_PAL:
+		actual_vm = omap_dss_pal_vm;
+		break;
+	case VENC_MODE_NTSC:
+		actual_vm = omap_dss_ntsc_vm;
+		break;
+	}
+
 	/* Reset WSS data when the TV standard changes. */
-	if (memcmp(&venc.vm, vm, sizeof(*vm)))
+	if (memcmp(&venc.vm, &actual_vm, sizeof(actual_vm)))
 		venc.wss_data = 0;
 
-	venc.vm = *vm;
+	venc.vm = actual_vm;
 
 	dispc_set_tv_pclk(13500000);
 
@@ -562,13 +599,13 @@ static int venc_check_timings(struct omap_dss_device *dssdev,
 {
 	DSSDBG("venc_check_timings\n");
 
-	if (memcmp(&omap_dss_pal_vm, vm, sizeof(*vm)) == 0)
+	switch (venc_get_videomode(vm)) {
+	case VENC_MODE_PAL:
+	case VENC_MODE_NTSC:
 		return 0;
-
-	if (memcmp(&omap_dss_ntsc_vm, vm, sizeof(*vm)) == 0)
-		return 0;
-
-	return -EINVAL;
+	default:
+		return -EINVAL;
+	}
 }
 
 static void venc_get_timings(struct omap_dss_device *dssdev,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/omap: fix i886 work-around

[Tomi Valkeinen]

7d267f068a8b4944d52e8b0ae4c8fcc1c1c5c5ba ("drm/omap: work-around for
errata i886") changed how the PLL dividers and multipliers are
calculated. While the new way should work fine for all the PLLs, it
breaks omap5 PLLs. The issues seen are rather odd: seemed that the
output clock rate is half of what we asked. It is unclear what's causing
there issues.

As a work-around this patch adds a "errata_i886" flag, which is set only
for DRA7's PLLs, and the PLL setup is done according to that flag.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dss.h       |  3 +++
 drivers/gpu/drm/omapdrm/dss/pll.c       | 29 ++++++++++++++++++++---------
 drivers/gpu/drm/omapdrm/dss/video-pll.c |  2 ++
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
index 5dd29c98143a..8ea3daa9aed9 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.h
+++ b/drivers/gpu/drm/omapdrm/dss/dss.h
@@ -174,6 +174,9 @@ struct dss_pll_hw {
 	bool has_freqsel;
 	bool has_selfreqdco;
 	bool has_refsel;
+
+	/* DRA7 errata i886: use high N & M to avoid jitter */
+	bool errata_i886;
 };
 
 struct dss_pll {
diff --git a/drivers/gpu/drm/omapdrm/dss/pll.c b/drivers/gpu/drm/omapdrm/dss/pll.c
index 5e221302768b..9d9d9d42009b 100644
--- a/drivers/gpu/drm/omapdrm/dss/pll.c
+++ b/drivers/gpu/drm/omapdrm/dss/pll.c
@@ -215,8 +215,8 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned long clkin,
 		dss_pll_calc_func func, void *data)
 {
 	const struct dss_pll_hw *hw = pll->hw;
-	int n, n_min, n_max;
-	int m, m_min, m_max;
+	int n, n_start, n_stop, n_inc;
+	int m, m_start, m_stop, m_inc;
 	unsigned long fint, clkdco;
 	unsigned long pll_hw_max;
 	unsigned long fint_hw_min, fint_hw_max;
@@ -226,22 +226,33 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned long clkin,
 	fint_hw_min = hw->fint_min;
 	fint_hw_max = hw->fint_max;
 
-	n_min = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
-	n_max = min((unsigned)(clkin / fint_hw_min), hw->n_max);
+	n_start = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
+	n_stop = min((unsigned)(clkin / fint_hw_min), hw->n_max);
+	n_inc = 1;
+
+	if (hw->errata_i886) {
+		swap(n_start, n_stop);
+		n_inc = -1;
+	}
 
 	pll_max = pll_max ? pll_max : ULONG_MAX;
 
-	/* Try to find high N & M to avoid jitter (DRA7 errata i886) */
-	for (n = n_max; n >= n_min; --n) {
+	for (n = n_start; n != n_stop; n += n_inc) {
 		fint = clkin / n;
 
-		m_min = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
+		m_start = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
 				1ul);
-		m_max = min3((unsigned)(pll_max / fint / 2),
+		m_stop = min3((unsigned)(pll_max / fint / 2),
 				(unsigned)(pll_hw_max / fint / 2),
 				hw->m_max);
+		m_inc = 1;
+
+		if (hw->errata_i886) {
+			swap(m_start, m_stop);
+			m_inc = -1;
+		}
 
-		for (m = m_max; m >= m_min; --m) {
+		for (m = m_start; m != m_stop; m += m_inc) {
 			clkdco = 2 * m * fint;
 
 			if (func(n, m, fint, clkdco, data))
diff --git a/drivers/gpu/drm/omapdrm/dss/video-pll.c b/drivers/gpu/drm/omapdrm/dss/video-pll.c
index 7429de928d4e..8201ecf826d9 100644
--- a/drivers/gpu/drm/omapdrm/dss/video-pll.c
+++ b/drivers/gpu/drm/omapdrm/dss/video-pll.c
@@ -131,6 +131,8 @@ static const struct dss_pll_hw dss_dra7_video_pll_hw = {
 	.mX_lsb[3] = 5,
 
 	.has_refsel = true,
+
+	.errata_i886 = true,
 };
 
 struct dss_pll *dss_video_pll_init(struct platform_device *pdev, int id,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/omap: fix i886 work-around

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tuesday 13 Jun 2017 12:02:10 Tomi Valkeinen wrote:
> 7d267f068a8b4944d52e8b0ae4c8fcc1c1c5c5ba ("drm/omap: work-around for
> errata i886") changed how the PLL dividers and multipliers are
> calculated. While the new way should work fine for all the PLLs, it
> breaks omap5 PLLs. The issues seen are rather odd: seemed that the
> output clock rate is half of what we asked. It is unclear what's causing
> there issues.

Does this patch result in PLL parameters for half of the expected rate, or 
does it compute the expected parameters, with the PLL producing half of the 
expected rate for an unknown reason ?

> As a work-around this patch adds a "errata_i886" flag, which is set only
> for DRA7's PLLs, and the PLL setup is done according to that flag.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dss.h       |  3 +++
>  drivers/gpu/drm/omapdrm/dss/pll.c       | 29 ++++++++++++++++++++---------
>  drivers/gpu/drm/omapdrm/dss/video-pll.c |  2 ++
>  3 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h
> b/drivers/gpu/drm/omapdrm/dss/dss.h index 5dd29c98143a..8ea3daa9aed9 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.h
> @@ -174,6 +174,9 @@ struct dss_pll_hw {
>  	bool has_freqsel;
>  	bool has_selfreqdco;
>  	bool has_refsel;
> +
> +	/* DRA7 errata i886: use high N & M to avoid jitter */
> +	bool errata_i886;
>  };
> 
>  struct dss_pll {
> diff --git a/drivers/gpu/drm/omapdrm/dss/pll.c
> b/drivers/gpu/drm/omapdrm/dss/pll.c index 5e221302768b..9d9d9d42009b 100644
> --- a/drivers/gpu/drm/omapdrm/dss/pll.c
> +++ b/drivers/gpu/drm/omapdrm/dss/pll.c
> @@ -215,8 +215,8 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned
> long clkin, dss_pll_calc_func func, void *data)
>  {
>  	const struct dss_pll_hw *hw = pll->hw;
> -	int n, n_min, n_max;
> -	int m, m_min, m_max;
> +	int n, n_start, n_stop, n_inc;
> +	int m, m_start, m_stop, m_inc;
>  	unsigned long fint, clkdco;
>  	unsigned long pll_hw_max;
>  	unsigned long fint_hw_min, fint_hw_max;
> @@ -226,22 +226,33 @@ bool dss_pll_calc_a(const struct dss_pll *pll,
> unsigned long clkin, fint_hw_min = hw->fint_min;
>  	fint_hw_max = hw->fint_max;
> 
> -	n_min = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
> -	n_max = min((unsigned)(clkin / fint_hw_min), hw->n_max);
> +	n_start = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
> +	n_stop = min((unsigned)(clkin / fint_hw_min), hw->n_max);
> +	n_inc = 1;
> +
> +	if (hw->errata_i886) {
> +		swap(n_start, n_stop);
> +		n_inc = -1;
> +	}
> 
>  	pll_max = pll_max ? pll_max : ULONG_MAX;
> 
> -	/* Try to find high N & M to avoid jitter (DRA7 errata i886) */
> -	for (n = n_max; n >= n_min; --n) {
> +	for (n = n_start; n != n_stop; n += n_inc) {
>  		fint = clkin / n;
> 
> -		m_min = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
> +		m_start = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
>  				1ul);
> -		m_max = min3((unsigned)(pll_max / fint / 2),
> +		m_stop = min3((unsigned)(pll_max / fint / 2),
>  				(unsigned)(pll_hw_max / fint / 2),
>  				hw->m_max);
> +		m_inc = 1;
> +
> +		if (hw->errata_i886) {
> +			swap(m_start, m_stop);
> +			m_inc = -1;
> +		}
> 
> -		for (m = m_max; m >= m_min; --m) {
> +		for (m = m_start; m != m_stop; m += m_inc) {
>  			clkdco = 2 * m * fint;
> 
>  			if (func(n, m, fint, clkdco, data))
> diff --git a/drivers/gpu/drm/omapdrm/dss/video-pll.c
> b/drivers/gpu/drm/omapdrm/dss/video-pll.c index 7429de928d4e..8201ecf826d9
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/video-pll.c
> +++ b/drivers/gpu/drm/omapdrm/dss/video-pll.c
> @@ -131,6 +131,8 @@ static const struct dss_pll_hw dss_dra7_video_pll_hw = {
> .mX_lsb[3] = 5,
> 
>  	.has_refsel = true,
> +
> +	.errata_i886 = true,
>  };
> 
>  struct dss_pll *dss_video_pll_init(struct platform_device *pdev, int id,

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/omap: fix i886 work-around

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 866 bytes --]



On 07/08/17 14:47, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tuesday 13 Jun 2017 12:02:10 Tomi Valkeinen wrote:
>> 7d267f068a8b4944d52e8b0ae4c8fcc1c1c5c5ba ("drm/omap: work-around for
>> errata i886") changed how the PLL dividers and multipliers are
>> calculated. While the new way should work fine for all the PLLs, it
>> breaks omap5 PLLs. The issues seen are rather odd: seemed that the
>> output clock rate is half of what we asked. It is unclear what's causing
>> there issues.
> 
> Does this patch result in PLL parameters for half of the expected rate, or 
> does it compute the expected parameters, with the PLL producing half of the 
> expected rate for an unknown reason ?

The PLL parameters look fine, and inside the HW limits, afaics, but the
produced clock rate seems to be about half.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/omap: misc fixes

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [170613 02:06]:
> Hi,
> 
> Here's three fixes for omapdrm:
> 
> - fix synclost flood on omap3
> - fix analog tv-out videomode check
> - fix DSI PLL setup
> 
> Tony, Aaro, Nikolaus, can you see if these fix the issues you're seeing?

Seems to get rid of the warnings I was seeing on n900, but can't
check the LCD on it as it's in a remote rack.

Regards,

Tony
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/omap: misc fixes

[H. Nikolaus Schaller]

Hi Tomi,

> Am 13.06.2017 um 11:02 schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
> 
> Hi,
> 
> Here's three fixes for omapdrm:
> 
> - fix synclost flood on omap3
> - fix analog tv-out videomode check
> - fix DSI PLL setup
> 
> Tony, Aaro, Nikolaus, can you see if these fix the issues you're seeing?

I have reverted the hackfixes we have here

and added your patches and both, GTA04 (OMAP3+DPI+VENC) and Pyra (OMAP5+MIPI+HDMI) boot
without (new) warnings and I have working LCD on both.

I could not test X11/HDMI on the Pyra because I am in the middle of testing updates
to my Rootfs (and X11 does not start due to a segfault).

BR,
Nikolaus

> 
> Tomi
> 
> Tomi Valkeinen (3):
>  drm/omap: work-around for omap3 display enable
>  drm/omap: fix analog tv-out modecheck
>  drm/omap: fix i886 work-around
> 
> drivers/gpu/drm/omapdrm/dss/dispc.c     | 14 +++++++
> drivers/gpu/drm/omapdrm/dss/dss.h       |  3 ++
> drivers/gpu/drm/omapdrm/dss/pll.c       | 29 ++++++++++-----
> drivers/gpu/drm/omapdrm/dss/venc.c      | 65 ++++++++++++++++++++++++++-------
> drivers/gpu/drm/omapdrm/dss/video-pll.c |  2 +
> 5 files changed, 90 insertions(+), 23 deletions(-)
> 
> -- 
> 2.7.4
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel