On Mon, Feb 18, 2013 at 07:00:27PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>
> Some HDMI registers can be used for SDVO, so saying "HDMIB" should be
> the same as saying "SDVOB" for a given HW generation. This was not
> true and led to confusions and even a regression.
>
> Previously we had:
> - SDVO{B,C} defined as the Gen3+ registers
> - HDMI{B,C,D} and PCH_SDVOB defined as the PCH registers
>
> But now:
> - SDVO{B,C} became GEN3_SDVO{B,C} on SDVO code
> - SDVO{B,C} became GEN4_HDMI{B,C} on HDMI code
> - HDMI{B,C,D} became PCH_HDMI{B,C,D}
> - PCH_SDVOB is still the same thing
>
> v2: Rebase (v1 was sent in May 2012).
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
I think we still have a bit of ugly left in here, especially that the
register bit definitions are splattered all over irks me a bit. What about
moving the HDMI stuff up to the SDVO definitions and giving the HDMI bits
consisten HDMI_ prefixes? Imo there's no point in adding duplicate
#defines for all the SDVO_ bits we use in intel_hdmi.c ...
-Daniel
> ---
> drivers/gpu/drm/i915/i915_reg.h | 19 ++++++++-------
> drivers/gpu/drm/i915/intel_display.c | 42 ++++++++++++++++++----------------
> drivers/gpu/drm/i915/intel_sdvo.c | 22 +++++++++---------
> 3 files changed, 42 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9e5844b..cd31af2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1680,8 +1680,9 @@
> #define SDVOB_HOTPLUG_INT_STATUS_I915 (1 << 6)
>
> /* SDVO port control */
> -#define SDVOB 0x61140
> -#define SDVOC 0x61160
> +#define GEN3_SDVOB 0x61140
> +#define GEN3_SDVOC 0x61160
> +#define PCH_SDVOB 0xe1140
> #define SDVO_ENABLE (1 << 31)
> #define SDVO_PIPE_B_SELECT (1 << 30)
> #define SDVO_STALL_SELECT (1 << 29)
> @@ -3979,8 +3980,12 @@
> #define FDI_PLL_CTL_1 0xfe000
> #define FDI_PLL_CTL_2 0xfe004
>
> -/* or SDVOB */
> -#define HDMIB 0xe1140
> +/* The same register may be used for SDVO or HDMI */
> +#define GEN4_HDMIB GEN3_SDVOB
> +#define GEN4_HDMIC GEN3_SDVOC
> +#define PCH_HDMIB PCH_SDVOB
> +#define PCH_HDMIC 0xe1150
> +#define PCH_HDMID 0xe1160
> #define PORT_ENABLE (1 << 31)
> #define TRANSCODER(pipe) ((pipe) << 30)
> #define TRANSCODER_CPT(pipe) ((pipe) << 29)
> @@ -4001,12 +4006,6 @@
> #define HSYNC_ACTIVE_HIGH (1 << 3)
> #define PORT_DETECTED (1 << 2)
>
> -/* PCH SDVOB multiplex with HDMIB */
> -#define PCH_SDVOB HDMIB
> -
> -#define HDMIC 0xe1150
> -#define HDMID 0xe1160
> -
> #define PCH_LVDS 0xe1180
> #define LVDS_DETECTED (1 << 1)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6eb3882..744db70 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1419,9 +1419,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
> "PCH LVDS enabled on transcoder %c, should be disabled\n",
> pipe_name(pipe));
>
> - assert_pch_hdmi_disabled(dev_priv, pipe, HDMIB);
> - assert_pch_hdmi_disabled(dev_priv, pipe, HDMIC);
> - assert_pch_hdmi_disabled(dev_priv, pipe, HDMID);
> + assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB);
> + assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC);
> + assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
> }
>
> /**
> @@ -8323,20 +8323,20 @@ static void intel_setup_outputs(struct drm_device *dev)
> if (has_edp_a(dev))
> intel_dp_init(dev, DP_A, PORT_A);
>
> - if (I915_READ(HDMIB) & PORT_DETECTED) {
> + if (I915_READ(PCH_HDMIB) & PORT_DETECTED) {
> /* PCH SDVOB multiplex with HDMIB */
> found = intel_sdvo_init(dev, PCH_SDVOB, true);
> if (!found)
> - intel_hdmi_init(dev, HDMIB, PORT_B);
> + intel_hdmi_init(dev, PCH_HDMIB, PORT_B);
> if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED))
> intel_dp_init(dev, PCH_DP_B, PORT_B);
> }
>
> - if (I915_READ(HDMIC) & PORT_DETECTED)
> - intel_hdmi_init(dev, HDMIC, PORT_C);
> + if (I915_READ(PCH_HDMIC) & PORT_DETECTED)
> + intel_hdmi_init(dev, PCH_HDMIC, PORT_C);
>
> - if (!dpd_is_edp && I915_READ(HDMID) & PORT_DETECTED)
> - intel_hdmi_init(dev, HDMID, PORT_D);
> + if (!dpd_is_edp && I915_READ(PCH_HDMID) & PORT_DETECTED)
> + intel_hdmi_init(dev, PCH_HDMID, PORT_D);
>
> if (I915_READ(PCH_DP_C) & DP_DETECTED)
> intel_dp_init(dev, PCH_DP_C, PORT_C);
> @@ -8348,24 +8348,26 @@ static void intel_setup_outputs(struct drm_device *dev)
> if (I915_READ(VLV_DISPLAY_BASE + DP_C) & DP_DETECTED)
> intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C, PORT_C);
>
> - if (I915_READ(VLV_DISPLAY_BASE + SDVOB) & PORT_DETECTED) {
> - intel_hdmi_init(dev, VLV_DISPLAY_BASE + SDVOB, PORT_B);
> + if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIB) & PORT_DETECTED) {
> + intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIB,
> + PORT_B);
> if (I915_READ(VLV_DISPLAY_BASE + DP_B) & DP_DETECTED)
> intel_dp_init(dev, VLV_DISPLAY_BASE + DP_B, PORT_B);
> }
>
> - if (I915_READ(VLV_DISPLAY_BASE + SDVOC) & PORT_DETECTED)
> - intel_hdmi_init(dev, VLV_DISPLAY_BASE + SDVOC, PORT_C);
> + if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & PORT_DETECTED)
> + intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
> + PORT_C);
>
> } else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
> bool found = false;
>
> - if (I915_READ(SDVOB) & SDVO_DETECTED) {
> + if (I915_READ(GEN3_SDVOB) & SDVO_DETECTED) {
> DRM_DEBUG_KMS("probing SDVOB\n");
> - found = intel_sdvo_init(dev, SDVOB, true);
> + found = intel_sdvo_init(dev, GEN3_SDVOB, true);
> if (!found && SUPPORTS_INTEGRATED_HDMI(dev)) {
> DRM_DEBUG_KMS("probing HDMI on SDVOB\n");
> - intel_hdmi_init(dev, SDVOB, PORT_B);
> + intel_hdmi_init(dev, GEN4_HDMIB, PORT_B);
> }
>
> if (!found && SUPPORTS_INTEGRATED_DP(dev)) {
> @@ -8376,16 +8378,16 @@ static void intel_setup_outputs(struct drm_device *dev)
>
> /* Before G4X SDVOC doesn't have its own detect register */
>
> - if (I915_READ(SDVOB) & SDVO_DETECTED) {
> + if (I915_READ(GEN3_SDVOB) & SDVO_DETECTED) {
> DRM_DEBUG_KMS("probing SDVOC\n");
> - found = intel_sdvo_init(dev, SDVOC, false);
> + found = intel_sdvo_init(dev, GEN3_SDVOC, false);
> }
>
> - if (!found && (I915_READ(SDVOC) & SDVO_DETECTED)) {
> + if (!found && (I915_READ(GEN3_SDVOC) & SDVO_DETECTED)) {
>
> if (SUPPORTS_INTEGRATED_HDMI(dev)) {
> DRM_DEBUG_KMS("probing HDMI on SDVOC\n");
> - intel_hdmi_init(dev, SDVOC, PORT_C);
> + intel_hdmi_init(dev, GEN4_HDMIC, PORT_C);
> }
> if (SUPPORTS_INTEGRATED_DP(dev)) {
> DRM_DEBUG_KMS("probing DP_C\n");
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index f01063a..7d94db8 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -246,11 +246,11 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo *intel_sdvo, u32 val)
> return;
> }
>
> - if (intel_sdvo->sdvo_reg == SDVOB) {
> - cval = I915_READ(SDVOC);
> - } else {
> - bval = I915_READ(SDVOB);
> - }
> + if (intel_sdvo->sdvo_reg == GEN3_SDVOB)
> + cval = I915_READ(GEN3_SDVOC);
> + else
> + bval = I915_READ(GEN3_SDVOB);
> +
> /*
> * Write the registers twice for luck. Sometimes,
> * writing them only once doesn't appear to 'stick'.
> @@ -258,10 +258,10 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo *intel_sdvo, u32 val)
> */
> for (i = 0; i < 2; i++)
> {
> - I915_WRITE(SDVOB, bval);
> - I915_READ(SDVOB);
> - I915_WRITE(SDVOC, cval);
> - I915_READ(SDVOC);
> + I915_WRITE(GEN3_SDVOB, bval);
> + I915_READ(GEN3_SDVOB);
> + I915_WRITE(GEN3_SDVOC, cval);
> + I915_READ(GEN3_SDVOC);
> }
> }
>
> @@ -1182,10 +1182,10 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
> } else {
> sdvox = I915_READ(intel_sdvo->sdvo_reg);
> switch (intel_sdvo->sdvo_reg) {
> - case SDVOB:
> + case GEN3_SDVOB:
> sdvox &= SDVOB_PRESERVE_MASK;
> break;
> - case SDVOC:
> + case GEN3_SDVOC:
> sdvox &= SDVOC_PRESERVE_MASK;
> break;
> }
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
