Russell King and Sebastian Hasselbarth had proposed some very good changes for the tda998x HDMI encoder driver. But when those changes were tested on BeagleBone Black against the tilcdc driver many modes would no longer display correctly. After analyzing the sync signals from the TI lcd contoller to the nxp it is apparent that the hsync/vsync's are not rising at the same time as per the VESA spec and this is causing the HDMI encoder to get messed up and failing to lock correctly.
This series of patches should be applied on top of:
Russell King's rmk's Dove DRM/TDA19988 Cubox driver series
Sebastian Hasselbarth's drm/i2c: tda998x: fix sync generation and calculation
I have done as much of the change as I can in the tilcdc driver but there is a small unavoidable change in the tda998x driver. However I have been careful not to break anything from the Dove drivers perspective. It would be great if somebody can test on Cubox and confirm that.
This patch set inverts the hsync signal coming from the tilcdc so the NXP is kept happy and then shifts the output to the right to compensate for the sync timing issues. Display modes from the NXP have been verified using a HDMI analyzer and are reporting correct timings at the output stage.
Hopefully this will allow the dove/tda driver changes to progress now that were blocked as per this discussion: http://lists.freedesktop.org/archives/dri-devel/2013-July/040900.html
Darren Etheridge (2): drm/i2c/tda998x prepare for tilcdc sync workaround drm/tilcdc fixup mode to workaound sync for tda998x
drivers/gpu/drm/i2c/tda998x_drv.c | 10 ++++++++++ drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 ++++++- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 21 ++++++++++++++++++++- 3 files changed, 36 insertions(+), 2 deletions(-)
Add necessary support for flipping the hsync signal and shifting the display to the right by a certain number of pixels. Changes only take effect if adjusted_mode is changed in fixup function.
Signed-off-by: Darren Etheridge detheridge@ti.com --- drivers/gpu/drm/i2c/tda998x_drv.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index e384b59..ad870e6 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -778,6 +778,16 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, n_line = mode->vtotal;
ref_pix = 3 + mode->hsync_start - mode->hdisplay; + + /* + * handle issue on TILCDC where it is outputing + * non-VESA compliant sync signals the workaround + * forces us to invert the HSYNC, so need to adjust display to + * the left by hskew pixels, provided by the tilcdc driver + */ + if (adjusted_mode->flags & DRM_MODE_FLAG_HSKEW) + ref_pix += adjusted_mode->hskew; + de_pix_s = mode->htotal - mode->hdisplay; de_pix_e = de_pix_s + mode->hdisplay; hs_pix_s = mode->hsync_start - mode->hdisplay;
Add a fixup function that will flip the hsync priority and add a hskew value that is used to shift the tda998x to the right by a variable number of pixels depending on the mode. This works around an issue with the sync timings that tilcdc is outputing.
Signed-off-by: Darren Etheridge detheridge@ti.com --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 ++++++- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 21 ++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 6118651..1c10bf1 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -377,7 +377,12 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc, else tilcdc_clear(dev, LCDC_RASTER_TIMING_2_REG, LCDC_SYNC_EDGE);
- if (mode->flags & DRM_MODE_FLAG_NHSYNC) + /* + * use value from adjusted_mode here as this might have been + * changed as part of the fixup for NXP TDA998x to solve the + * issue where tilcdc timings are not VESA compliant + */ + if (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC) tilcdc_set(dev, LCDC_RASTER_TIMING_2_REG, LCDC_INVERT_HSYNC); else tilcdc_clear(dev, LCDC_RASTER_TIMING_2_REG, LCDC_INVERT_HSYNC); diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c index dfffaf0..d5197a3 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c @@ -73,13 +73,32 @@ static void slave_encoder_prepare(struct drm_encoder *encoder) tilcdc_crtc_set_panel_info(encoder->crtc, &slave_info); }
+static bool slave_encoder_fixup(struct drm_encoder *encoder, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + adjusted_mode->hskew = mode->hsync_end - mode->hsync_start; + adjusted_mode->flags |= DRM_MODE_FLAG_HSKEW; + + if (mode->flags & DRM_MODE_FLAG_NHSYNC) { + adjusted_mode->flags |= DRM_MODE_FLAG_PHSYNC; + adjusted_mode->flags &= ~DRM_MODE_FLAG_NHSYNC; + } else { + adjusted_mode->flags |= DRM_MODE_FLAG_NHSYNC; + adjusted_mode->flags &= ~DRM_MODE_FLAG_PHSYNC; + } + + return drm_i2c_encoder_mode_fixup(encoder, mode, adjusted_mode); +} + + static const struct drm_encoder_funcs slave_encoder_funcs = { .destroy = slave_encoder_destroy, };
static const struct drm_encoder_helper_funcs slave_encoder_helper_funcs = { .dpms = drm_i2c_encoder_dpms, - .mode_fixup = drm_i2c_encoder_mode_fixup, + .mode_fixup = slave_encoder_fixup, .prepare = slave_encoder_prepare, .commit = drm_i2c_encoder_commit, .mode_set = drm_i2c_encoder_mode_set,
On Thu, Jul 25, 2013 at 2:32 PM, Darren Etheridge detheridge@ti.com wrote:
Russell King and Sebastian Hasselbarth had proposed some very good changes for the tda998x HDMI encoder driver. But when those changes were tested on BeagleBone Black against the tilcdc driver many modes would no longer display correctly. After analyzing the sync signals from the TI lcd contoller to the nxp it is apparent that the hsync/vsync's are not rising at the same time as per the VESA spec and this is causing the HDMI encoder to get messed up and failing to lock correctly.
This series of patches should be applied on top of:
Russell King's rmk's Dove DRM/TDA19988 Cubox driver series
Sebastian Hasselbarth's drm/i2c: tda998x: fix sync generation and calculation
I have done as much of the change as I can in the tilcdc driver but there is a small unavoidable change in the tda998x driver. However I have been careful not to break anything from the Dove drivers perspective. It would be great if somebody can test on Cubox and confirm that.
This patch set inverts the hsync signal coming from the tilcdc so the NXP is kept happy and then shifts the output to the right to compensate for the sync timing issues. Display modes from the NXP have been verified using a HDMI analyzer and are reporting correct timings at the output stage.
Hopefully this will allow the dove/tda driver changes to progress now that were blocked as per this discussion: http://lists.freedesktop.org/archives/dri-devel/2013-July/040900.html
Good find Darren! The patches look good to me from a quick review. It would be good to get a tested-by from someone on cubox, but it is good that we finally found the issue so that we can unblock further tda998x development.
For the series:
Reviewed-by: Rob Clark robdclark@gmail.com
Darren Etheridge (2): drm/i2c/tda998x prepare for tilcdc sync workaround drm/tilcdc fixup mode to workaound sync for tda998x
drivers/gpu/drm/i2c/tda998x_drv.c | 10 ++++++++++ drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 ++++++- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 21 ++++++++++++++++++++- 3 files changed, 36 insertions(+), 2 deletions(-)
On 07/25/2013 09:32 PM, Rob Clark wrote:
On Thu, Jul 25, 2013 at 2:32 PM, Darren Etheridge detheridge@ti.com wrote:
Russell King and Sebastian Hasselbarth had proposed some very good changes for the tda998x HDMI encoder driver. But when those changes were tested on BeagleBone Black against the tilcdc driver many modes would no longer display correctly. After analyzing the sync signals from the TI lcd contoller to the nxp it is apparent that the hsync/vsync's are not rising at the same time as per the VESA spec and this is causing the HDMI encoder to get messed up and failing to lock correctly.
This series of patches should be applied on top of:
Russell King's rmk's Dove DRM/TDA19988 Cubox driver series
Sebastian Hasselbarth's drm/i2c: tda998x: fix sync generation and calculation
I have done as much of the change as I can in the tilcdc driver but there is a small unavoidable change in the tda998x driver. However I have been careful not to break anything from the Dove drivers perspective. It would be great if somebody can test on Cubox and confirm that.
This patch set inverts the hsync signal coming from the tilcdc so the NXP is kept happy and then shifts the output to the right to compensate for the sync timing issues. Display modes from the NXP have been verified using a HDMI analyzer and are reporting correct timings at the output stage.
Hopefully this will allow the dove/tda driver changes to progress now that were blocked as per this discussion: http://lists.freedesktop.org/archives/dri-devel/2013-July/040900.html
Good find Darren! The patches look good to me from a quick review. It would be good to get a tested-by from someone on cubox, but it is good that we finally found the issue so that we can unblock further tda998x development.
Thanks to Darren, I can now test tda998x sync changes on both Cubox and Beaglebone Black. I don't think the changes will affect Armada DRM in any way, as it is not setting adjusted_mode.
I will put a scope on AM335x/TDA998x sync signals to fully understand the issues tilcdc has, but I can think of a flag for TDA998x to always accept falling input hsync/vsync and tilcdc to supply that sync. That will maybe allow us to get rid of hskew workaround.
As far as I understand the issue, tilcdc always aligns VS with the rising HS edge? If so, enforcing positive HS/VS on tilcdc and telling TDA998x that it is always positive, may be a cleaner workaround. TDA998x can invert input and output sync independently.
In any way, it will take some time to get a working setup. If you are happy with the patches, I can still send some follow-up patches later.
Sebastian
On 07/25/2013 09:32 PM, Rob Clark wrote:
On Thu, Jul 25, 2013 at 2:32 PM, Darren Etheridge detheridge@ti.com wrote:
[...]
This patch set inverts the hsync signal coming from the tilcdc so the NXP is kept happy and then shifts the output to the right to compensate for the sync timing issues. Display modes from the NXP have been verified using a HDMI analyzer and are reporting correct timings at the output stage.
Hopefully this will allow the dove/tda driver changes to progress now that were blocked as per this discussion: http://lists.freedesktop.org/archives/dri-devel/2013-July/040900.html
Good find Darren! The patches look good to me from a quick review. It would be good to get a tested-by from someone on cubox, but it is good that we finally found the issue so that we can unblock further tda998x development.
Darren,
I now fully understand the issues of AM335x's LCD controller and your fix for it. I suggest to clarify the comments you added to tilcdc to allow others to understand it more quickly.
Actually, the LCD controller always aligns vsync to the second edge of hsync, which will never give VESA-compliant sync. The (elegant) workaround you are proposing is to align both rising edges, so at least TDA998x can sync on those with some hskew added. Lucky you that it ignores hsync length but only looks for rising HS/VS edges ;)
Should we prepare a new patch set comprising the following patches?
Russell King: drm/i2c: nxp-tda998x: fix EDID reading on TDA19988 devices drm/i2c: nxp-tda998x: ensure VIP output mux is properly set drm/i2c: nxp-tda998x: fix npix/nline programming drm/i2c: nxp-tda998x: prepare for video input configuration drm/i2c: nxp-tda998x: add video and audio input configuration
Sebastian Hesselbarth: drm/i2c: tda998x: fix sync generation and calculation
Darren Etheridge: drm/i2c/tda998x prepare for tilcdc sync workaround drm/tilcdc fixup mode to workaound sync for tda998x
Or do we keep them separated and possibly resend them if David cannot find them anymore?
Sebastian
On Wed, Jul 31, 2013 at 10:21:20PM +0200, Sebastian Hesselbarth wrote:
Should we prepare a new patch set comprising the following patches?
Russell King: drm/i2c: nxp-tda998x: fix EDID reading on TDA19988 devices drm/i2c: nxp-tda998x: ensure VIP output mux is properly set drm/i2c: nxp-tda998x: fix npix/nline programming drm/i2c: nxp-tda998x: prepare for video input configuration drm/i2c: nxp-tda998x: add video and audio input configuration
Sebastian Hesselbarth: drm/i2c: tda998x: fix sync generation and calculation
Unless you've changed this one since I tested it, it needs fixes which basically revert the vsync changes back to the original code.
Sebastian Hesselbarth sebastian.hesselbarth@gmail.com wrote on Wed [2013-Jul-31 22:21:20 +0200]:
Darren,
I now fully understand the issues of AM335x's LCD controller and your fix for it. I suggest to clarify the comments you added to tilcdc to allow others to understand it more quickly.
Sebastian, Thanks for looking at my proposed changes, you understand this sync stuff very well so I appreciate your input that this is actually an acceptable workaround.
Actually, the LCD controller always aligns vsync to the second edge of hsync, which will never give VESA-compliant sync. The (elegant) workaround you are proposing is to align both rising edges, so at least TDA998x can sync on those with some hskew added. Lucky you that it ignores hsync length but only looks for rising HS/VS edges ;)
Yes we definitely got lucky with this one, good thing the NXP supported that reference pixel position, as I was out of options from the lcd controller side of things to adjust the horizontal position.
Should we prepare a new patch set comprising the following patches?
Russell King: drm/i2c: nxp-tda998x: fix EDID reading on TDA19988 devices drm/i2c: nxp-tda998x: ensure VIP output mux is properly set drm/i2c: nxp-tda998x: fix npix/nline programming drm/i2c: nxp-tda998x: prepare for video input configuration drm/i2c: nxp-tda998x: add video and audio input configuration
Sebastian Hesselbarth: drm/i2c: tda998x: fix sync generation and calculation
Darren Etheridge: drm/i2c/tda998x prepare for tilcdc sync workaround drm/tilcdc fixup mode to workaound sync for tda998x
Or do we keep them separated and possibly resend them if David cannot find them anymore?
I vote we submit a complete series that we can all test, there were quite a lot of versions of things in flight at the same time so I am sure David would appreciate a consolidated version. The only thing I have not tested is audio support, but as the original driver did not have that anyway I don't consider it blocking if it is working for CuBox.
Darren
On Thu, Aug 1, 2013 at 10:29 AM, Darren Etheridge detheridge@ti.com wrote:
Sebastian Hesselbarth sebastian.hesselbarth@gmail.com wrote on Wed [2013-Jul-31 22:21:20 +0200]:
Darren,
I now fully understand the issues of AM335x's LCD controller and your fix for it. I suggest to clarify the comments you added to tilcdc to allow others to understand it more quickly.
Sebastian, Thanks for looking at my proposed changes, you understand this sync stuff very well so I appreciate your input that this is actually an acceptable workaround.
Actually, the LCD controller always aligns vsync to the second edge of hsync, which will never give VESA-compliant sync. The (elegant) workaround you are proposing is to align both rising edges, so at least TDA998x can sync on those with some hskew added. Lucky you that it ignores hsync length but only looks for rising HS/VS edges ;)
Yes we definitely got lucky with this one, good thing the NXP supported that reference pixel position, as I was out of options from the lcd controller side of things to adjust the horizontal position.
Should we prepare a new patch set comprising the following patches?
Russell King: drm/i2c: nxp-tda998x: fix EDID reading on TDA19988 devices drm/i2c: nxp-tda998x: ensure VIP output mux is properly set drm/i2c: nxp-tda998x: fix npix/nline programming drm/i2c: nxp-tda998x: prepare for video input configuration drm/i2c: nxp-tda998x: add video and audio input configuration
Sebastian Hesselbarth: drm/i2c: tda998x: fix sync generation and calculation
Darren Etheridge: drm/i2c/tda998x prepare for tilcdc sync workaround drm/tilcdc fixup mode to workaound sync for tda998x
Or do we keep them separated and possibly resend them if David cannot find them anymore?
I vote we submit a complete series that we can all test, there were quite a lot of versions of things in flight at the same time so I am sure David would appreciate a consolidated version.
I'm sure Dave would appreciate it if someone set up a tree w/ all the patches consolidated and sent a pull req.
I can help with that if needed, but it probably makes more sense for someone who is using a beagle-bone and/or CuBox on a more regular basis to do this
BR, -R
The only thing I have not tested is audio support, but as the original driver did not have that anyway I don't consider it blocking if it is working for CuBox.
Darren
dri-devel@lists.freedesktop.org
-
Darren Etheridge -
Rob Clark -
Russell King - ARM Linux -
Sebastian Hesselbarth