This patch uses always the adjusted video mode instead of a mix of
original and adjusted mode.
Signed-off-by: Jean-Francois Moine <[email protected]>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 66 +++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index b688801..5d82301 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -773,7 +773,7 @@ tda998x_encoder_mode_valid(struct drm_encoder *encoder,
static void
tda998x_encoder_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ struct drm_display_mode *adj_mode)
{
struct tda998x_priv *priv = to_tda998x_priv(encoder);
uint16_t ref_pix, ref_line, n_pix, n_line;
@@ -802,13 +802,13 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
* So we add +1 to all horizontal and vertical register values,
* plus an additional +3 for REFPIX as we are using RGB input only.
*/
- n_pix = mode->htotal;
- n_line = mode->vtotal;
+ n_pix = adj_mode->htotal;
+ n_line = adj_mode->vtotal;
- hs_pix_e = mode->hsync_end - mode->hdisplay;
- hs_pix_s = mode->hsync_start - mode->hdisplay;
- de_pix_e = mode->htotal;
- de_pix_s = mode->htotal - mode->hdisplay;
+ hs_pix_e = adj_mode->hsync_end - adj_mode->hdisplay;
+ hs_pix_s = adj_mode->hsync_start - adj_mode->hdisplay;
+ de_pix_e = adj_mode->htotal;
+ de_pix_s = adj_mode->htotal - adj_mode->hdisplay;
ref_pix = 3 + hs_pix_s;
/*
@@ -816,37 +816,38 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
* those to adjust the position of the rising VS edge by adding
* HSKEW to ref_pix.
*/
- if (adjusted_mode->flags & DRM_MODE_FLAG_HSKEW)
- ref_pix += adjusted_mode->hskew;
+ if (adj_mode->flags & DRM_MODE_FLAG_HSKEW)
+ ref_pix += adj_mode->hskew;
- if ((mode->flags & DRM_MODE_FLAG_INTERLACE) == 0) {
- ref_line = 1 + mode->vsync_start - mode->vdisplay;
- vwin1_line_s = mode->vtotal - mode->vdisplay - 1;
- vwin1_line_e = vwin1_line_s + mode->vdisplay;
+ if ((adj_mode->flags & DRM_MODE_FLAG_INTERLACE) == 0) {
+ ref_line = 1 + adj_mode->vsync_start - adj_mode->vdisplay;
+ vwin1_line_s = adj_mode->vtotal - adj_mode->vdisplay - 1;
+ vwin1_line_e = vwin1_line_s + adj_mode->vdisplay;
vs1_pix_s = vs1_pix_e = hs_pix_s;
- vs1_line_s = mode->vsync_start - mode->vdisplay;
+ vs1_line_s = adj_mode->vsync_start - adj_mode->vdisplay;
vs1_line_e = vs1_line_s +
- mode->vsync_end - mode->vsync_start;
+ adj_mode->vsync_end - adj_mode->vsync_start;
vwin2_line_s = vwin2_line_e = 0;
vs2_pix_s = vs2_pix_e = 0;
vs2_line_s = vs2_line_e = 0;
} else {
- ref_line = 1 + (mode->vsync_start - mode->vdisplay)/2;
- vwin1_line_s = (mode->vtotal - mode->vdisplay)/2;
- vwin1_line_e = vwin1_line_s + mode->vdisplay/2;
+ ref_line = 1 + (adj_mode->vsync_start -
+ adj_mode->vdisplay)/2;
+ vwin1_line_s = (adj_mode->vtotal - adj_mode->vdisplay)/2;
+ vwin1_line_e = vwin1_line_s + adj_mode->vdisplay/2;
vs1_pix_s = vs1_pix_e = hs_pix_s;
- vs1_line_s = (mode->vsync_start - mode->vdisplay)/2;
+ vs1_line_s = (adj_mode->vsync_start - adj_mode->vdisplay)/2;
vs1_line_e = vs1_line_s +
- (mode->vsync_end - mode->vsync_start)/2;
- vwin2_line_s = vwin1_line_s + mode->vtotal/2;
- vwin2_line_e = vwin2_line_s + mode->vdisplay/2;
- vs2_pix_s = vs2_pix_e = hs_pix_s + mode->htotal/2;
- vs2_line_s = vs1_line_s + mode->vtotal/2 ;
+ (adj_mode->vsync_end - adj_mode->vsync_start)/2;
+ vwin2_line_s = vwin1_line_s + adj_mode->vtotal/2;
+ vwin2_line_e = vwin2_line_s + adj_mode->vdisplay/2;
+ vs2_pix_s = vs2_pix_e = hs_pix_s + adj_mode->htotal/2;
+ vs2_line_s = vs1_line_s + adj_mode->vtotal/2 ;
vs2_line_e = vs2_line_s +
- (mode->vsync_end - mode->vsync_start)/2;
+ (adj_mode->vsync_end - adj_mode->vsync_start)/2;
}
- div = 148500 / mode->clock;
+ div = 148500 / adj_mode->clock;
/* mute the audio FIFO: */
reg_set(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO);
@@ -896,9 +897,9 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
* TDA19988 requires high-active sync at input stage,
* so invert low-active sync provided by master encoder here
*/
- if (mode->flags & DRM_MODE_FLAG_NHSYNC)
+ if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC)
reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_H_TGL);
- if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+ if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC)
reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_V_TGL);
/*
@@ -906,9 +907,9 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
* revert input stage toggled sync at output stage
*/
reg = TBG_CNTRL_1_TGL_EN;
- if (mode->flags & DRM_MODE_FLAG_NHSYNC)
+ if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC)
reg |= TBG_CNTRL_1_H_TGL;
- if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+ if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC)
reg |= TBG_CNTRL_1_V_TGL;
reg_write(priv, REG_TBG_CNTRL_1, reg);
@@ -949,11 +950,10 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
reg_write(priv, REG_ENC_CNTRL, ENC_CNTRL_CTL_CODE(1));
reg_set(priv, REG_TX33, TX33_HDMI);
- tda998x_write_avi(priv, adjusted_mode);
+ tda998x_write_avi(priv, adj_mode);
if (priv->params.audio_cfg)
- tda998x_configure_audio(priv, adjusted_mode,
- &priv->params);
+ tda998x_configure_audio(priv, adj_mode, &priv->params);
}
}
--
1.8.5.3
On Sun, Jan 19, 2014 at 07:58:40PM +0100, Jean-Francois Moine wrote:
> This patch uses always the adjusted video mode instead of a mix of
> original and adjusted mode.
>
> Signed-off-by: Jean-Francois Moine <[email protected]>
Nothing obviously wrong and appears to work, thanks.
Acked-by: Russell King <[email protected]>
Tested-by: Russell King <[email protected]>
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
Jean-Francois Moine <[email protected]> wrote on Sun [2014-Jan-19 19:58:40 +0100]:
> This patch uses always the adjusted video mode instead of a mix of
> original and adjusted mode.
>
> Signed-off-by: Jean-Francois Moine <[email protected]>
> ---
> drivers/gpu/drm/i2c/tda998x_drv.c | 66 +++++++++++++++++++--------------------
> 1 file changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index b688801..5d82301 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -773,7 +773,7 @@ tda998x_encoder_mode_valid(struct drm_encoder *encoder,
> static void
> tda998x_encoder_mode_set(struct drm_encoder *encoder,
> struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> + struct drm_display_mode *adj_mode)
> {
> struct tda998x_priv *priv = to_tda998x_priv(encoder);
> uint16_t ref_pix, ref_line, n_pix, n_line;
> @@ -802,13 +802,13 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
> * So we add +1 to all horizontal and vertical register values,
> * plus an additional +3 for REFPIX as we are using RGB input only.
> */
> - n_pix = mode->htotal;
> - n_line = mode->vtotal;
> + n_pix = adj_mode->htotal;
> + n_line = adj_mode->vtotal;
>
> - hs_pix_e = mode->hsync_end - mode->hdisplay;
> - hs_pix_s = mode->hsync_start - mode->hdisplay;
> - de_pix_e = mode->htotal;
> - de_pix_s = mode->htotal - mode->hdisplay;
> + hs_pix_e = adj_mode->hsync_end - adj_mode->hdisplay;
> + hs_pix_s = adj_mode->hsync_start - adj_mode->hdisplay;
> + de_pix_e = adj_mode->htotal;
> + de_pix_s = adj_mode->htotal - adj_mode->hdisplay;
> ref_pix = 3 + hs_pix_s;
>
> /*
> @@ -816,37 +816,38 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
> * those to adjust the position of the rising VS edge by adding
> * HSKEW to ref_pix.
> */
> - if (adjusted_mode->flags & DRM_MODE_FLAG_HSKEW)
> - ref_pix += adjusted_mode->hskew;
> + if (adj_mode->flags & DRM_MODE_FLAG_HSKEW)
> + ref_pix += adj_mode->hskew;
>
> - if ((mode->flags & DRM_MODE_FLAG_INTERLACE) == 0) {
> - ref_line = 1 + mode->vsync_start - mode->vdisplay;
> - vwin1_line_s = mode->vtotal - mode->vdisplay - 1;
> - vwin1_line_e = vwin1_line_s + mode->vdisplay;
> + if ((adj_mode->flags & DRM_MODE_FLAG_INTERLACE) == 0) {
> + ref_line = 1 + adj_mode->vsync_start - adj_mode->vdisplay;
> + vwin1_line_s = adj_mode->vtotal - adj_mode->vdisplay - 1;
> + vwin1_line_e = vwin1_line_s + adj_mode->vdisplay;
> vs1_pix_s = vs1_pix_e = hs_pix_s;
> - vs1_line_s = mode->vsync_start - mode->vdisplay;
> + vs1_line_s = adj_mode->vsync_start - adj_mode->vdisplay;
> vs1_line_e = vs1_line_s +
> - mode->vsync_end - mode->vsync_start;
> + adj_mode->vsync_end - adj_mode->vsync_start;
> vwin2_line_s = vwin2_line_e = 0;
> vs2_pix_s = vs2_pix_e = 0;
> vs2_line_s = vs2_line_e = 0;
> } else {
> - ref_line = 1 + (mode->vsync_start - mode->vdisplay)/2;
> - vwin1_line_s = (mode->vtotal - mode->vdisplay)/2;
> - vwin1_line_e = vwin1_line_s + mode->vdisplay/2;
> + ref_line = 1 + (adj_mode->vsync_start -
> + adj_mode->vdisplay)/2;
> + vwin1_line_s = (adj_mode->vtotal - adj_mode->vdisplay)/2;
> + vwin1_line_e = vwin1_line_s + adj_mode->vdisplay/2;
> vs1_pix_s = vs1_pix_e = hs_pix_s;
> - vs1_line_s = (mode->vsync_start - mode->vdisplay)/2;
> + vs1_line_s = (adj_mode->vsync_start - adj_mode->vdisplay)/2;
> vs1_line_e = vs1_line_s +
> - (mode->vsync_end - mode->vsync_start)/2;
> - vwin2_line_s = vwin1_line_s + mode->vtotal/2;
> - vwin2_line_e = vwin2_line_s + mode->vdisplay/2;
> - vs2_pix_s = vs2_pix_e = hs_pix_s + mode->htotal/2;
> - vs2_line_s = vs1_line_s + mode->vtotal/2 ;
> + (adj_mode->vsync_end - adj_mode->vsync_start)/2;
> + vwin2_line_s = vwin1_line_s + adj_mode->vtotal/2;
> + vwin2_line_e = vwin2_line_s + adj_mode->vdisplay/2;
> + vs2_pix_s = vs2_pix_e = hs_pix_s + adj_mode->htotal/2;
> + vs2_line_s = vs1_line_s + adj_mode->vtotal/2 ;
> vs2_line_e = vs2_line_s +
> - (mode->vsync_end - mode->vsync_start)/2;
> + (adj_mode->vsync_end - adj_mode->vsync_start)/2;
> }
>
> - div = 148500 / mode->clock;
> + div = 148500 / adj_mode->clock;
>
> /* mute the audio FIFO: */
> reg_set(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO);
> @@ -896,9 +897,9 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
> * TDA19988 requires high-active sync at input stage,
> * so invert low-active sync provided by master encoder here
> */
> - if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> + if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC)
> reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_H_TGL);
> - if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> + if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC)
> reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_V_TGL);
>
Using the adj_mode->flags breaks a workaround I had done on BeagleBone Black
(tilcdc + tda998x) to resolve an issue with out of spec syncs from the
tlcdc. I invert the HSYNC in adj_mode->flags but don't want the tda998x to
really know that I am doing that so I use adj_mode in the tilcdc driver, and
mode here in the tda998x driver. The theory being adj_mode contains whatever
workarounds I need to do for the driving device and mode has the pristine
values that I want to send to the monitor. I would need to look if there is a
different way to solve this as I am guessing you are actually using adj_mode in
the manner it was intended.
Otherwise this patch series is working on BeagleBone Black - I have only tried
video so far (not audio).
Darren
> /*
> @@ -906,9 +907,9 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
> * revert input stage toggled sync at output stage
> */
> reg = TBG_CNTRL_1_TGL_EN;
> - if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> + if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC)
> reg |= TBG_CNTRL_1_H_TGL;
> - if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> + if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC)
> reg |= TBG_CNTRL_1_V_TGL;
> reg_write(priv, REG_TBG_CNTRL_1, reg);
>
> @@ -949,11 +950,10 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
> reg_write(priv, REG_ENC_CNTRL, ENC_CNTRL_CTL_CODE(1));
> reg_set(priv, REG_TX33, TX33_HDMI);
>
> - tda998x_write_avi(priv, adjusted_mode);
> + tda998x_write_avi(priv, adj_mode);
>
> if (priv->params.audio_cfg)
> - tda998x_configure_audio(priv, adjusted_mode,
> - &priv->params);
> + tda998x_configure_audio(priv, adj_mode, &priv->params);
> }
> }
>
> --
> 1.8.5.3
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, 23 Jan 2014 17:29:07 -0600
Darren Etheridge <[email protected]> wrote:
> > @@ -896,9 +897,9 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
> > * TDA19988 requires high-active sync at input stage,
> > * so invert low-active sync provided by master encoder here
> > */
> > - if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> > + if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC)
> > reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_H_TGL);
> > - if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> > + if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC)
> > reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_V_TGL);
> >
>
> Using the adj_mode->flags breaks a workaround I had done on BeagleBone Black
> (tilcdc + tda998x) to resolve an issue with out of spec syncs from the
> tlcdc. I invert the HSYNC in adj_mode->flags but don't want the tda998x to
> really know that I am doing that so I use adj_mode in the tilcdc driver, and
> mode here in the tda998x driver. The theory being adj_mode contains whatever
> workarounds I need to do for the driving device and mode has the pristine
> values that I want to send to the monitor. I would need to look if there is a
> different way to solve this as I am guessing you are actually using adj_mode in
> the manner it was intended.
No. In fact, I just wanted the function to use only one mode.
Looking at the other drivers, it seems that they don't touch the
adjusted_mode, so, for the Cubox, mode and adjusted_mode have same
values.
I will do an other patch so that you will not have to touch the tilcdc
driver.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
Jean-Francois Moine <[email protected]> wrote on Tue [2014-Jan-28 18:12:18 +0100]:
> On Thu, 23 Jan 2014 17:29:07 -0600
> Darren Etheridge <[email protected]> wrote:
>
> > > @@ -896,9 +897,9 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
> > > * TDA19988 requires high-active sync at input stage,
> > > * so invert low-active sync provided by master encoder here
> > > */
> > > - if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> > > + if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC)
> > > reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_H_TGL);
> > > - if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> > > + if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC)
> > > reg_set(priv, REG_VIP_CNTRL_3, VIP_CNTRL_3_V_TGL);
> > >
> >
> > Using the adj_mode->flags breaks a workaround I had done on BeagleBone Black
> > (tilcdc + tda998x) to resolve an issue with out of spec syncs from the
> > tlcdc. I invert the HSYNC in adj_mode->flags but don't want the tda998x to
> > really know that I am doing that so I use adj_mode in the tilcdc driver, and
> > mode here in the tda998x driver. The theory being adj_mode contains whatever
> > workarounds I need to do for the driving device and mode has the pristine
> > values that I want to send to the monitor. I would need to look if there is a
> > different way to solve this as I am guessing you are actually using adj_mode in
> > the manner it was intended.
>
> No. In fact, I just wanted the function to use only one mode.
>
> Looking at the other drivers, it seems that they don't touch the
> adjusted_mode, so, for the Cubox, mode and adjusted_mode have same
> values.
>
> I will do an other patch so that you will not have to touch the tilcdc
> driver.
>
Thanks, that would certainly be the easiest path to avoid the regression given
the other drivers that use tda998x don't currently use adj_mode. However I
don't disagree with your reasoning, it would make sense to only use values
from one of the strctures and not a mix and match. Anybody looking at only
the tda998x driver would be confused :) I'll work on finding a different
solution for this.
> --
> Ken ar c'henta? | ** Breizh ha Linux atav! **
> Jef | http://moinejf.free.fr/