On Thu, Mar 06, 2014 at 05:04:26PM +0100, Denis Carikli wrote:
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-di.c
> b/drivers/staging/imx-drm/ipu-v3/ipu-di.c
> index 849b3e1..5d273c1 100644
> --- a/drivers/staging/imx-drm/ipu-v3/ipu-di.c
> +++ b/drivers/staging/imx-drm/ipu-v3/ipu-di.c
> @@ -595,8 +595,12 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct
> ipu_di_signal_cfg *sig)
> }
> }
>
> - if (sig->clk_pol)
> - di_gen |= DI_GEN_POLARITY_DISP_CLK;
> + if (sig->set_mask & SET_CLK_POL) {
> + if (sig->clk_pol)
> + di_gen |= DI_GEN_POLARITY_DISP_CLK;
> + else
> + di_gen &= ~DI_GEN_POLARITY_DISP_CLK;
> + }
>
> ipu_di_write(di, di_gen, DI_GENERAL);
>
> @@ -606,8 +610,13 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct
> ipu_di_signal_cfg *sig)
> reg = ipu_di_read(di, DI_POL);
> reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
>
> - if (sig->enable_pol)
> - reg |= DI_POL_DRDY_POLARITY_15;
> + if (sig->set_mask & SET_DE_POL) {
> + if (sig->enable_pol)
> + reg |= DI_POL_DRDY_POLARITY_15;
> + else
> + reg &= ~DI_POL_DRDY_POLARITY_15;
> + }
> +
> if (sig->data_pol)
> reg |= DI_POL_DRDY_DATA_POLARITY;
>
> diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c
> b/drivers/staging/imx-drm/ipuv3-crtc.c
> index f506075..71f757f 100644
> --- a/drivers/staging/imx-drm/ipuv3-crtc.c
> +++ b/drivers/staging/imx-drm/ipuv3-crtc.c
> @@ -157,8 +157,22 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
> if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> sig_cfg.Vsync_pol = 1;
>
> - sig_cfg.enable_pol = 1;
> - sig_cfg.clk_pol = 0;
> + if (mode->private_flags & IMXDRM_MODE_FLAG_DE_HIGH) {
> + sig_cfg.enable_pol = 1;
> + sig_cfg.set_mask |= SET_DE_POL;
> + } else if (mode->private_flags & IMXDRM_MODE_FLAG_DE_LOW) {
> + sig_cfg.enable_pol = 0;
> + sig_cfg.set_mask |= SET_DE_POL;
> + }
> +
> + if (mode->private_flags & IMXDRM_MODE_FLAG_PIXDATA_POSEDGE) {
> + sig_cfg.clk_pol = 1;
> + sig_cfg.set_mask |= SET_CLK_POL;
> + } else if (mode->private_flags & IMXDRM_MODE_FLAG_PIXDATA_NEGEDGE) {
> + sig_cfg.clk_pol = 0;
> + sig_cfg.set_mask |= SET_CLK_POL;
> + }
So how does this work for other displays, for example, HDMI, where we need
enable_pol=1 and clk_pol=0 ?
>From what I can see, we end up with sig_cfg.enable_pol=0, sig_cfg.clk_pol=0,
sig_cfg.set_mask=0.
What we end up with for enable_pol is this:
reg = ipu_di_read(di, DI_POL);
reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
- if (sig->enable_pol)
- reg |= DI_POL_DRDY_POLARITY_15;
+ if (sig->set_mask & SET_DE_POL) {
+ if (sig->enable_pol)
+ reg |= DI_POL_DRDY_POLARITY_15;
+ else
+ reg &= ~DI_POL_DRDY_POLARITY_15;
+ }
which is no different from:
reg = ipu_di_read(di, DI_POL);
reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
if (sig->set_mask & SET_DE_POL && sig->enable_pol)
reg |= DI_POL_DRDY_POLARITY_15;
because even with SET_DE_POL unset, we still end up clearing the bit.
Similar seems to happen with the clock polarity as well - in order for
that bit to be set, buth the set_mask and the clk_pol but must be set.
Dovetailing in to my previous reply, if we want to do this, maybe we
should convert clk_pol and enable_pol to be a tristate (can be an
u8 or enum). Essentially, it has three values: preserve, active high,
active low.
However, one of the things that really worries me here is the "preserve"
action - what if it's not correctly set initially, and we don't have
anything specifying either polarity, which will happen if the only
encoder/connector you have is imx-hdmi.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
