On Tue, Jun 10, 2014 at 09:58:54AM -0300, Fabio Estevam wrote:
> Booting the kernel with the HDMI cable connected (no image is seen on
> HDMI, only on LVDS):
Reformatting a bit:
> disp 0: panel size = 1920 x 1080
> Clocks: IPU 264000000Hz DI 24000000Hz Needed 138500000Hz
> IPU clock can give 132000000 with divider 2, error -4.3%
> Want 138500000Hz IPU 264000000Hz DI 138500000Hz using DI, 138500000Hz
> disp 1: panel size = 1024 x 768
> Clocks: IPU 264000000Hz DI 64999999Hz Needed 65000000Hz
> Want 65000000Hz IPU 264000000Hz DI 64999999Hz using DI, 64999999Hz
>
> After cable removal:
> disp 0: panel size = 1024 x 768
> Clocks: IPU 264000000Hz DI 64999999Hz Needed 65000000Hz
> Want 65000000Hz IPU 264000000Hz DI 64999999Hz using DI, 64999999Hz
So here we can see that the clock for DI0 got changed beneath it from
138.5MHz to 65MHz, probably when the DI1 clock was changed.
> After cable re-insertion (image is seen on both HDMI and LVDS):
> disp 0: panel size = 1920 x 1080
> Clocks: IPU 264000000Hz DI 64999999Hz Needed 138500000Hz
> IPU clock can give 132000000 with divider 2, error -4.3%
> Want 138500000Hz IPU 264000000Hz DI 129999997Hz using DI, 129999997Hz
> disp 1: panel size = 1024 x 768
> Clocks: IPU 264000000Hz DI 64999999Hz Needed 65000000Hz
> Want 65000000Hz IPU 264000000Hz DI 64999999Hz using DI, 64999999Hz
The strange thing here is that DI0, when asking for 138.5MHz, only gets
130MHz (a multiple of DI1's frequency) yet when DI1 reduced it to 65MHz,
the CCF just obliged.
Let's go back to the basic requirements for LDB, because I think there's
a bug here.
1. the LDB serial clock must be 7x the LDB DI clock.
2. for single/split/separate mode, the IPU DI clock must be the same
and synchronous with the LDB DI clock.
3. for dual mode, the IPU DI clock must be twice the LDB DI clock, and
must be synchronised with the LDB DI clock.
(1) is provided for us via the CCM clock tree - there is already a /7
divider between the LDB serial clock and the LDB DI clock:
LM --+-------- LDB serial
`- /7 -- LDB DI clock
where LM is the LDB clock muxer.
I'm going to leave case (3) for the time being, because at the moment, I
can't see how to achieve it given the clock tree structure that the iMX6
CPUs offer - I'm going to make the assumption at the present time that
this mode is not supported.
The easiest way to achieve (2) is to set the IPU DI clock mux to select
the LDB DI clock directly:
LM --+---------------- LDB serial
`- /7 -+-------- LDB DI clock
`- IM -- IPU DI clock
where 'M' is the IPU DI clock muxer. However, we're currently setting
this up as:
LM --+---------------- LDB serial
`- /7 -+-------- LDB DI clock
IPM --- /N ---- IM --- IPU DI clock
and hoping that the LDB and IPU DI clocks are appropriately synchronised.
This is the bug I refer to above.
Finally, for HDMI, we need:
PLL5 -- IPM -- IM -- IPU DI clock
What this implies is that we need control of the IPU DI clock muxer (IM)
and each DI needs to know what kind of display bridge(s) are connected
to it, so that the appropriate parent(s) can be selected.
We can get a little closer to that (and clean up the code) with the patch
below. It gives us some of the information we have really been wanting to
know all along in ipu_crtc_mode_set(), that is which kinds of bridges are
connected to the DI.
What we /really/ need to solve the above problem is exactly which bridges
are connected as well - whether it's LDB0 or LDB1, and also how to get at
the clocks to be able to control that mux. I'm at a loss how best to do
that given the complexity of iMX6 DT, and I don't think we want to add
these to the IPU/DI node as the IPU/DI isn't really connected to these
clocks - it's connected to the IM mux which is internal to the CCM.
I think this patch also should help Denis Carikli, as the polarity of
the clock signal (etc) also depends on the type(s) of encoders attached
to the DI. We really should fail if we end up with encoders with
incompatible requirements trying to be bound to a single DI.
(side note: I'd really like to get rid of imx_drm_panel_format*()...)
In any case, to fix this I think we're looking at changing the DT stuff
again, and as the IPU code is moving out of drivers/staging, this is going
to become increasing difficult...
drivers/staging/imx-drm/imx-drm-core.c | 3 +--
drivers/staging/imx-drm/imx-drm.h | 2 +-
drivers/staging/imx-drm/ipuv3-crtc.c | 40 +++++++++++++++++++---------------
3 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index 7da0cad27b49..c538c82f8a32 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -115,8 +115,7 @@ int imx_drm_panel_format_pins(struct drm_encoder *encoder,
helper = &imx_crtc->imx_drm_helper_funcs;
if (helper->set_interface_pix_fmt)
return helper->set_interface_pix_fmt(encoder->crtc,
- encoder->encoder_type, interface_pix_fmt,
- hsync_pin, vsync_pin);
+ interface_pix_fmt, hsync_pin, vsync_pin);
return 0;
}
EXPORT_SYMBOL_GPL(imx_drm_panel_format_pins);
diff --git a/drivers/staging/imx-drm/imx-drm.h b/drivers/staging/imx-drm/imx-drm.h
index a322bac55414..0bf8e5eb76e3 100644
--- a/drivers/staging/imx-drm/imx-drm.h
+++ b/drivers/staging/imx-drm/imx-drm.h
@@ -17,7 +17,7 @@ int imx_drm_crtc_id(struct imx_drm_crtc *crtc);
struct imx_drm_crtc_helper_funcs {
int (*enable_vblank)(struct drm_crtc *crtc);
void (*disable_vblank)(struct drm_crtc *crtc);
- int (*set_interface_pix_fmt)(struct drm_crtc *crtc, u32 encoder_type,
+ int (*set_interface_pix_fmt)(struct drm_crtc *crtc,
u32 pix_fmt, int hsync_pin, int vsync_pin);
const struct drm_crtc_helper_funcs *crtc_helper_funcs;
const struct drm_crtc_funcs *crtc_funcs;
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index 47bec5e17358..796e9bef15f0 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -51,7 +51,6 @@ struct ipu_crtc {
struct drm_framebuffer *newfb;
int irq;
u32 interface_pix_fmt;
- unsigned long di_clkflags;
int di_hsync_pin;
int di_vsync_pin;
};
@@ -146,10 +145,13 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
int x, int y,
struct drm_framebuffer *old_fb)
{
+ struct drm_device *dev = crtc->dev;
+ struct drm_encoder *encoder;
struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
- int ret;
struct ipu_di_signal_cfg sig_cfg = {};
+ unsigned long encoder_types = 0;
u32 out_pixel_fmt;
+ int ret;
dev_dbg(ipu_crtc->dev, "%s: mode->hdisplay: %d\n", __func__,
mode->hdisplay);
@@ -165,6 +167,24 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
if (mode->flags & DRM_MODE_FLAG_PVSYNC)
sig_cfg.Vsync_pol = 1;
+ list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
+ if (encoder->crtc == crtc)
+ encoder_types |= BIT(encoder->encoder_type);
+
+ dev_dbg(ipu_crtc->dev, "%s: attached to encoder types 0x%x\n",
+ __func__, encoder_types);
+
+ /*
+ * If we have DAC, TVDAC or LDB, then we need the IPU DI clock
+ * to be the same as the LDB DI clock.
+ */
+ if (encoder_types & (BIT(DRM_MODE_ENCODER_DAC) |
+ BIT(DRM_MODE_ENCODER_TVDAC) |
+ BIT(DRM_MODE_ENCODER_LVDS)))
+ sig_cfg.clkflags = IPU_DI_CLKMODE_SYNC | IPU_DI_CLKMODE_EXT;
+ else
+ sig_cfg.clkflags = 0;
+
sig_cfg.enable_pol = 1;
sig_cfg.clk_pol = 0;
sig_cfg.width = mode->hdisplay;
@@ -178,7 +198,6 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
sig_cfg.v_sync_width = mode->vsync_end - mode->vsync_start;
sig_cfg.v_end_width = mode->vsync_start - mode->vdisplay;
sig_cfg.pixelclock = mode->clock * 1000;
- sig_cfg.clkflags = ipu_crtc->di_clkflags;
sig_cfg.v_to_h_sync = 0;
@@ -277,7 +296,7 @@ static void ipu_disable_vblank(struct drm_crtc *crtc)
ipu_crtc->newfb = NULL;
}
-static int ipu_set_interface_pix_fmt(struct drm_crtc *crtc, u32 encoder_type,
+static int ipu_set_interface_pix_fmt(struct drm_crtc *crtc,
u32 pixfmt, int hsync_pin, int vsync_pin)
{
struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
@@ -286,19 +305,6 @@ static int ipu_set_interface_pix_fmt(struct drm_crtc *crtc, u32 encoder_type,
ipu_crtc->di_hsync_pin = hsync_pin;
ipu_crtc->di_vsync_pin = vsync_pin;
- switch (encoder_type) {
- case DRM_MODE_ENCODER_DAC:
- case DRM_MODE_ENCODER_TVDAC:
- case DRM_MODE_ENCODER_LVDS:
- ipu_crtc->di_clkflags = IPU_DI_CLKMODE_SYNC |
- IPU_DI_CLKMODE_EXT;
- break;
- case DRM_MODE_ENCODER_TMDS:
- case DRM_MODE_ENCODER_NONE:
- ipu_crtc->di_clkflags = 0;
- break;
- }
-
return 0;
}
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
