On Wed, 30 Jan 2013, Rodrigo Vivi rodrigo.vivi@gmail.com wrote:

drm_i915_private is getting bigger and bigger when adding new vbt stuff. So, the better way of getting drm_i915_private organized is to create an special structure for vbt stuff.

Reviewed-by: Jani Nikula jani.nikula@intel.com

Signed-off-by: Rodrigo Vivi rodrigo.vivi@gmail.com

drivers/gpu/drm/i915/i915_dma.c | 8 +-- drivers/gpu/drm/i915/i915_drv.h | 56 +++++++++++---------- drivers/gpu/drm/i915/intel_bios.c | 96 ++++++++++++++++++------------------ drivers/gpu/drm/i915/intel_crt.c | 4 +- drivers/gpu/drm/i915/intel_display.c | 16 +++--- drivers/gpu/drm/i915/intel_dp.c | 12 ++--- drivers/gpu/drm/i915/intel_lvds.c | 20 ++++---- drivers/gpu/drm/i915/intel_sdvo.c | 6 +-- drivers/gpu/drm/i915/intel_tv.c | 8 +-- 9 files changed, 116 insertions(+), 110 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 3f70178..e43711f 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1714,10 +1714,10 @@ int i915_driver_unload(struct drm_device *dev) * free the memory space allocated for the child device * config parsed from VBT */

• if (dev_priv->child_dev && dev_priv->child_dev_num) {
  

• 	kfree(dev_priv->child_dev);
  

• 	dev_priv->child_dev = NULL;
  

• 	dev_priv->child_dev_num = 0;
  

• if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {
  

• 	kfree(dev_priv->vbt.child_dev);
  

• 	dev_priv->vbt.child_dev = NULL;
  

• 	dev_priv->vbt.child_dev_num = 0;
  

  }

  vga_switcheroo_unregister_client(dev->pdev);

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3189034..e22f6bd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -665,6 +665,36 @@ struct intel_l3_parity { struct work_struct error_work; };

+struct intel_vbt_data {

• struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
• struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
• /* Feature bits */
• unsigned int int_tv_support:1;
• unsigned int lvds_dither:1;
• unsigned int lvds_vbt:1;
• unsigned int int_crt_support:1;
• unsigned int lvds_use_ssc:1;
• unsigned int display_clock_mode:1;
• int lvds_ssc_freq;
• unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
• /* eDP */
• int edp_rate;
• int edp_lanes;
• int edp_preemphasis;
• int edp_vswing;
• bool edp_initialized;
• bool edp_support;
• int edp_bpp;
• struct edp_power_seq edp_pps;
• int crt_ddc_pin;
• int child_dev_num;
• struct child_device_config *child_dev;

+};

typedef struct drm_i915_private { struct drm_device *dev; struct kmem_cache *slab; @@ -745,6 +775,7 @@ typedef struct drm_i915_private { struct intel_fbc_work *fbc_work;

struct intel_opregion opregion;

• struct intel_vbt_data vbt;

  /* overlay */ struct intel_overlay *overlay;

@@ -753,32 +784,9 @@ typedef struct drm_i915_private { /* LVDS info */ int backlight_level; /* restore backlight to this value */ bool backlight_enabled;

• struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */

• struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */

• /* Feature bits from the VBIOS */

• unsigned int int_tv_support:1;

• unsigned int lvds_dither:1;

• unsigned int lvds_vbt:1;

• unsigned int int_crt_support:1;

• unsigned int lvds_use_ssc:1;

• unsigned int display_clock_mode:1;

• int lvds_ssc_freq;

• unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */

• struct {

• int rate;
  

• int lanes;
  

• int preemphasis;
  

• int vswing;
  

• bool initialized;
  

• bool support;
  

• int bpp;
  

• struct edp_power_seq pps;
  

• } edp; bool no_aux_handshake;

• int crt_ddc_pin; struct drm_i915_fence_reg fence_regs[I915_MAX_NUM_FENCES]; /* assume 965 */ int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */ int num_fence_regs; /* 8 on pre-965, 16 otherwise */

@@ -928,8 +936,6 @@ typedef struct drm_i915_private { /* indicates the reduced downclock for LVDS*/ int lvds_downclock; u16 orig_clock;

• int child_dev_num;

• struct child_device_config *child_dev;

  bool mchbar_need_disable;

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 55ffba1..e145151 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -212,7 +212,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv, if (!lvds_options) return;

• dev_priv->lvds_dither = lvds_options->pixel_dither;

• dev_priv->vbt.lvds_dither = lvds_options->pixel_dither; if (lvds_options->panel_type == 0xff) return;

@@ -226,7 +226,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv, if (!lvds_lfp_data_ptrs) return;

• dev_priv->lvds_vbt = 1;

• dev_priv->vbt.lvds_vbt = 1;

  panel_dvo_timing = get_lvds_dvo_timing(lvds_lfp_data, lvds_lfp_data_ptrs,

@@ -238,7 +238,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,

fill_detail_timing_data(panel_fixed_mode, panel_dvo_timing);

• dev_priv->lfp_lvds_vbt_mode = panel_fixed_mode;

• dev_priv->vbt.lfp_lvds_vbt_mode = panel_fixed_mode;

  DRM_DEBUG_KMS("Found panel mode in BIOS VBT tables:\n"); drm_mode_debug_printmodeline(panel_fixed_mode);

@@ -274,9 +274,9 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv, /* check the resolution, just to be sure */ if (fp_timing->x_res == panel_fixed_mode->hdisplay && fp_timing->y_res == panel_fixed_mode->vdisplay) {

• 	dev_priv->bios_lvds_val = fp_timing->lvds_reg_val;
  

• 	dev_priv->vbt.bios_lvds_val = fp_timing->lvds_reg_val;
  DRM_DEBUG_KMS("VBT initial LVDS value %x\n",
  

• 		      dev_priv->bios_lvds_val);
  

• 		      dev_priv->vbt.bios_lvds_val);
  

  } }

} @@ -316,7 +316,7 @@ parse_sdvo_panel_data(struct drm_i915_private *dev_priv,

fill_detail_timing_data(panel_fixed_mode, dvo_timing + index);

• dev_priv->sdvo_lvds_vbt_mode = panel_fixed_mode;

• dev_priv->vbt.sdvo_lvds_vbt_mode = panel_fixed_mode;

  DRM_DEBUG_KMS("Found SDVO panel mode in BIOS VBT tables:\n"); drm_mode_debug_printmodeline(panel_fixed_mode);

@@ -345,18 +345,18 @@ parse_general_features(struct drm_i915_private *dev_priv,

general = find_section(bdb, BDB_GENERAL_FEATURES); if (general) {

• dev_priv->int_tv_support = general->int_tv_support;
  

• dev_priv->int_crt_support = general->int_crt_support;
  

• dev_priv->lvds_use_ssc = general->enable_ssc;
  

• dev_priv->lvds_ssc_freq =
  

• dev_priv->vbt.int_tv_support = general->int_tv_support;
  

• dev_priv->vbt.int_crt_support = general->int_crt_support;
  

• dev_priv->vbt.lvds_use_ssc = general->enable_ssc;
  

• dev_priv->vbt.lvds_ssc_freq =
  intel_bios_ssc_frequency(dev, general->ssc_freq);
  

• dev_priv->display_clock_mode = general->display_clock_mode;
  

• dev_priv->vbt.display_clock_mode = general->display_clock_mode;
  

  DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d\n",

• 	      dev_priv->int_tv_support,
  

• 	      dev_priv->int_crt_support,
  

• 	      dev_priv->lvds_use_ssc,
  

• 	      dev_priv->lvds_ssc_freq,
  

• 	      dev_priv->display_clock_mode);
  

• 	      dev_priv->vbt.int_tv_support,
  

• 	      dev_priv->vbt.int_crt_support,
  

• 	      dev_priv->vbt.lvds_use_ssc,
  

• 	      dev_priv->vbt.lvds_ssc_freq,
  

• 	      dev_priv->vbt.display_clock_mode);
  

  }

}

@@ -373,7 +373,7 @@ parse_general_definitions(struct drm_i915_private *dev_priv, int bus_pin = general->crt_ddc_gmbus_pin; DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin); if (intel_gmbus_is_port_valid(bus_pin))

• 		dev_priv->crt_ddc_pin = bus_pin;
  

• 		dev_priv->vbt.crt_ddc_pin = bus_pin;
  

  } else { DRM_DEBUG_KMS("BDB_GD too small (%d). Invalid.\n", block_size);

@@ -484,7 +484,7 @@ parse_driver_features(struct drm_i915_private *dev_priv,

if (SUPPORTS_EDP(dev) && driver->lvds_config == BDB_DRIVER_FEATURE_EDP)

• dev_priv->edp.support = 1;
  

• dev_priv->vbt.edp_support = 1;
  
  

  if (driver->dual_frequency) dev_priv->render_reclock_avail = true;

@@ -499,20 +499,20 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)

edp = find_section(bdb, BDB_EDP); if (!edp) {

• if (SUPPORTS_EDP(dev_priv->dev) && dev_priv->edp.support)
  

• if (SUPPORTS_EDP(dev_priv->dev) && dev_priv->vbt.edp_support)
  DRM_DEBUG_KMS("No eDP BDB found but eDP panel supported.\n");
  

  return; }

  switch ((edp->color_depth >> (panel_type * 2)) & 3) { case EDP_18BPP:

• dev_priv->edp.bpp = 18;
  

• dev_priv->vbt.edp_bpp = 18;
  

  break; case EDP_24BPP:

• dev_priv->edp.bpp = 24;
  

• dev_priv->vbt.edp_bpp = 24;
  

  break; case EDP_30BPP:

• dev_priv->edp.bpp = 30;
  

• dev_priv->vbt.edp_bpp = 30;
  

  break; }

@@ -520,48 +520,48 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb) edp_pps = &edp->power_seqs[panel_type]; edp_link_params = &edp->link_params[panel_type];

• dev_priv->edp.pps = *edp_pps;

• dev_priv->vbt.edp_pps = *edp_pps;

• dev_priv->edp.rate = edp_link_params->rate ? DP_LINK_BW_2_7 :

• dev_priv->vbt.edp_rate = edp_link_params->rate ? DP_LINK_BW_2_7 : DP_LINK_BW_1_62; switch (edp_link_params->lanes) { case 0:

• dev_priv->edp.lanes = 1;
  

• dev_priv->vbt.edp_lanes = 1;
  

  break; case 1:

• dev_priv->edp.lanes = 2;
  

• dev_priv->vbt.edp_lanes = 2;
  

  break; case 3: default:

• dev_priv->edp.lanes = 4;
  

• dev_priv->vbt.edp_lanes = 4;
  

  break; } switch (edp_link_params->preemphasis) { case 0:

• dev_priv->edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_0;
  

• dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_0;
  

  break; case 1:

• dev_priv->edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_3_5;
  

• dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_3_5;
  

  break; case 2:

• dev_priv->edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_6;
  

• dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_6;
  

  break; case 3:

• dev_priv->edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_9_5;
  

• dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_9_5;
  

  break; } switch (edp_link_params->vswing) { case 0:

• dev_priv->edp.vswing = DP_TRAIN_VOLTAGE_SWING_400;
  

• dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_400;
  

  break; case 1:

• dev_priv->edp.vswing = DP_TRAIN_VOLTAGE_SWING_600;
  

• dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_600;
  

  break; case 2:

• dev_priv->edp.vswing = DP_TRAIN_VOLTAGE_SWING_800;
  

• dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_800;
  

  break; case 3:

• dev_priv->edp.vswing = DP_TRAIN_VOLTAGE_SWING_1200;
  

• dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_1200;
  

  break; }

} @@ -609,13 +609,13 @@ parse_device_mapping(struct drm_i915_private *dev_priv, DRM_DEBUG_KMS("no child dev is parsed from VBT\n"); return; }

• dev_priv->child_dev = kcalloc(count, sizeof(*p_child), GFP_KERNEL);
• if (!dev_priv->child_dev) {

• dev_priv->vbt.child_dev = kcalloc(count, sizeof(*p_child), GFP_KERNEL);
• if (!dev_priv->vbt.child_dev) { DRM_DEBUG_KMS("No memory space for child device\n"); return; }

• dev_priv->child_dev_num = count;

• dev_priv->vbt.child_dev_num = count; count = 0; for (i = 0; i < child_device_num; i++) { p_child = &(p_defs->devices[i]);

@@ -623,7 +623,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv, /* skip the device block if device type is invalid */ continue; }

• child_dev_ptr = dev_priv->child_dev + count;
  

• child_dev_ptr = dev_priv->vbt.child_dev + count;
  

  count++; memcpy((void *)child_dev_ptr, (void *)p_child, sizeof(*p_child));

@@ -636,23 +636,23 @@ init_vbt_defaults(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv->dev;

• dev_priv->crt_ddc_pin = GMBUS_PORT_VGADDC;

• dev_priv->vbt.crt_ddc_pin = GMBUS_PORT_VGADDC;

  /* LFP panel data */

• dev_priv->lvds_dither = 1;
• dev_priv->lvds_vbt = 0;

• dev_priv->vbt.lvds_dither = 1;

• dev_priv->vbt.lvds_vbt = 0;

  /* SDVO panel data */

• dev_priv->sdvo_lvds_vbt_mode = NULL;

• dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;

  /* general features */

• dev_priv->int_tv_support = 1;
• dev_priv->int_crt_support = 1;

• dev_priv->vbt.int_tv_support = 1;

• dev_priv->vbt.int_crt_support = 1;

  /* Default to using SSC */

• dev_priv->lvds_use_ssc = 1;
• dev_priv->lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1);
• DRM_DEBUG_KMS("Set default to SSC at %dMHz\n", dev_priv->lvds_ssc_freq);

• dev_priv->vbt.lvds_use_ssc = 1;
• dev_priv->vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1);
• DRM_DEBUG_KMS("Set default to SSC at %dMHz\n", dev_priv->vbt.lvds_ssc_freq);

}

static int __init intel_no_opregion_vbt_callback(const struct dmi_system_id *id) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 71a5eba..3f30095 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -433,7 +433,7 @@ static bool intel_crt_detect_ddc(struct drm_connector *connector)

BUG_ON(crt->base.type != INTEL_OUTPUT_ANALOG);

• i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin);

• i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin); edid = intel_crt_get_edid(connector, i2c);

  if (edid) {

@@ -639,7 +639,7 @@ static int intel_crt_get_modes(struct drm_connector *connector) int ret; struct i2c_adapter *i2c;

• i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin);

• i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin); ret = intel_crt_ddc_get_modes(connector, i2c); if (ret || !IS_G4X(dev)) return ret;

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b35902e..9e4db94 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3984,7 +3984,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) { if (i915_panel_use_ssc >= 0) return i915_panel_use_ssc != 0;

• return dev_priv->lvds_use_ssc

• return dev_priv->vbt.lvds_use_ssc && !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);

}

@@ -4055,7 +4055,7 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,

if (intel_encoder->type == INTEL_OUTPUT_EDP) {
	/* Use VBT settings if we have an eDP panel */

• 	unsigned int edp_bpc = dev_priv->edp.bpp / 3;
  

• 	unsigned int edp_bpc = dev_priv->vbt.edp_bpp / 3;
  
  if (edp_bpc && edp_bpc < display_bpc) {
  	DRM_DEBUG_KMS("clamping display bpc (was %d) to eDP (%d)\n", display_bpc, edp_bpc);
  

@@ -4156,7 +4156,7 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors) refclk = vlv_get_refclk(crtc); } else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) && intel_panel_use_ssc(dev_priv) && num_connectors < 2) {

• refclk = dev_priv->lvds_ssc_freq * 1000;
  

• refclk = dev_priv->vbt.lvds_ssc_freq * 1000;
  

  DRM_DEBUG_KMS("using SSC reference clock of %d MHz\n", refclk / 1000); } else if (!IS_GEN2(dev)) {

@@ -4768,7 +4768,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) }

if (HAS_PCH_IBX(dev)) {

• has_ck505 = dev_priv->display_clock_mode;
  

• has_ck505 = dev_priv->vbt.display_clock_mode;
  

  can_ssc = has_ck505; } else { has_ck505 = false;

@@ -5049,8 +5049,8 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)

if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) { DRM_DEBUG_KMS("using SSC reference clock of %d MHz\n",

• 	      dev_priv->lvds_ssc_freq);
  

• return dev_priv->lvds_ssc_freq * 1000;
  

• 	      dev_priv->vbt.lvds_ssc_freq);
  

• return dev_priv->vbt.lvds_ssc_freq * 1000;
  

  }

  return 120000;

@@ -5399,7 +5399,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, factor = 21; if (is_lvds) { if ((intel_panel_use_ssc(dev_priv) &&

•      dev_priv->lvds_ssc_freq == 100) ||
  

•      dev_priv->vbt.lvds_ssc_freq == 100) ||
  

  intel_is_dual_link_lvds(dev)) factor = 25; } else if (is_sdvo && is_tv)

@@ -5512,7 +5512,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, /* determine panel color depth */ dither = intel_choose_pipe_bpp_dither(crtc, fb, &intel_crtc->bpp, adjusted_mode);

• if (is_lvds && dev_priv->lvds_dither)

• if (is_lvds && dev_priv->vbt.lvds_dither) dither = true;

  fp = clock.n << 16 | clock.m1 << 8 | clock.m2;

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1492706..cbd3236 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2609,11 +2609,11 @@ bool intel_dpd_is_edp(struct drm_device *dev) struct child_device_config *p_child; int i;

• if (!dev_priv->child_dev_num)

• if (!dev_priv->vbt.child_dev_num) return false;

• for (i = 0; i < dev_priv->child_dev_num; i++) {

• p_child = dev_priv->child_dev + i;
  

• for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {

• p_child = dev_priv->vbt.child_dev + i;
  
  

  if (p_child->dvo_port == PORT_IDPD && p_child->device_type == DEVICE_TYPE_eDP)

@@ -2677,7 +2677,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n", cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);

• vbt = dev_priv->edp.pps;

• vbt = dev_priv->vbt.edp_pps;

  /* Upper limits from eDP 1.3 spec. Note that we use the clunky units of

  • our hw here, which are all in 100usec. */

@@ -2886,8 +2886,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, }

/* fallback to VBT if available for eDP */

• if (!fixed_mode && dev_priv->lfp_lvds_vbt_mode) {
  

• 	fixed_mode = drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
  

• if (!fixed_mode && dev_priv->vbt.lfp_lvds_vbt_mode) {
  

• 	fixed_mode = drm_mode_duplicate(dev, dev_priv->vbt.lfp_lvds_vbt_mode);
  if (fixed_mode)
  	fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
  

  }

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 8c61876..d3be319 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -137,7 +137,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder) * special lvds dither control bit on pch-split platforms, dithering is * only controlled through the PIPECONF reg. */ if (INTEL_INFO(dev)->gen == 4) {

• if (dev_priv->lvds_dither)
  

• if (dev_priv->vbt.lvds_dither)
  temp |= LVDS_ENABLE_DITHER;
  

  else temp &= ~LVDS_ENABLE_DITHER;

@@ -454,7 +454,7 @@ out: }

/* Make sure pre-965 set dither correctly */

• if (INTEL_INFO(dev)->gen < 4 && dev_priv->lvds_dither)

• if (INTEL_INFO(dev)->gen < 4 && dev_priv->vbt.lvds_dither) pfit_control |= PANEL_8TO6_DITHER_ENABLE;

  if (pfit_control != lvds_encoder->pfit_control ||

@@ -920,11 +920,11 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; int i;

• if (!dev_priv->child_dev_num)

• if (!dev_priv->vbt.child_dev_num) return true;

• for (i = 0; i < dev_priv->child_dev_num; i++) {

• struct child_device_config *child = dev_priv->child_dev + i;
  

• for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {

• struct child_device_config *child = dev_priv->vbt.child_dev + i;
  
  

  /* If the device type is not LFP, continue.

  • We have to check both the new identifiers as well as the

@@ -1012,7 +1012,7 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder) */ val = I915_READ(lvds_encoder->reg); if (!(val & ~(LVDS_PIPE_MASK | LVDS_DETECTED)))

• val = dev_priv->bios_lvds_val;
  

• val = dev_priv->vbt.bios_lvds_val;
  
  

  return (val & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP;

} @@ -1069,7 +1069,7 @@ bool intel_lvds_init(struct drm_device *dev) if (HAS_PCH_SPLIT(dev)) { if ((I915_READ(PCH_LVDS) & LVDS_DETECTED) == 0) return false;

• if (dev_priv->edp.support) {
  

• if (dev_priv->vbt.edp_support) {
  DRM_DEBUG_KMS("disable LVDS for eDP support\n");
  return false;
  

  }

@@ -1190,11 +1190,11 @@ bool intel_lvds_init(struct drm_device *dev) }

/* Failed to get EDID, what about VBT? */

• if (dev_priv->lfp_lvds_vbt_mode) {

• if (dev_priv->vbt.lfp_lvds_vbt_mode) { DRM_DEBUG_KMS("using mode from VBT: ");

• drm_mode_debug_printmodeline(dev_priv->lfp_lvds_vbt_mode);
  

• drm_mode_debug_printmodeline(dev_priv->vbt.lfp_lvds_vbt_mode);
  
  

• fixed_mode = drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
  

• fixed_mode = drm_mode_duplicate(dev, dev_priv->vbt.lfp_lvds_vbt_mode);
  

  if (fixed_mode) { fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; goto out;

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index f01063a..4d0be3d 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1487,7 +1487,7 @@ intel_sdvo_get_analog_edid(struct drm_connector *connector)

return drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv,

• 				    dev_priv->crt_ddc_pin));
  

• 				    dev_priv->vbt.crt_ddc_pin));
  

}

static enum drm_connector_status @@ -1773,9 +1773,9 @@ static void intel_sdvo_get_lvds_modes(struct drm_connector *connector) goto end;

/* Fetch modes from VBT */

• if (dev_priv->sdvo_lvds_vbt_mode != NULL) {

• if (dev_priv->vbt.sdvo_lvds_vbt_mode != NULL) { newmode = drm_mode_duplicate(connector->dev,

• 			     dev_priv->sdvo_lvds_vbt_mode);
  

• 			     dev_priv->vbt.sdvo_lvds_vbt_mode);
  

  if (newmode != NULL) { /* Guarantee the mode is preferred */ newmode->type = (DRM_MODE_TYPE_PREFERRED |

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 984a113..500f053 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -1521,12 +1521,12 @@ static int tv_is_present_in_vbt(struct drm_device *dev) struct child_device_config *p_child; int i, ret;

• if (!dev_priv->child_dev_num)

• if (!dev_priv->vbt.child_dev_num) return 1;

  ret = 0;

• for (i = 0; i < dev_priv->child_dev_num; i++) {

• p_child = dev_priv->child_dev + i;
  

• for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {

• p_child = dev_priv->vbt.child_dev + i;
  

  /*
  • If the device type is not TV, continue.
  */

@@ -1564,7 +1564,7 @@ intel_tv_init(struct drm_device *dev) return; } /* Even if we have an encoder we may not have a connector */

• if (!dev_priv->int_tv_support)

• if (!dev_priv->vbt.int_tv_support) return;

  /*

-- 1.7.11.7

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi rodrigo.vivi@gmail.com wrote:

While old platforms had 3 transcoders and 3 pipes (1:1), HSW has 4 transcoders and 3 pipes. To avoid future mistakes transcoders must be used here instead of pipes even though it is working right now by coincidence.

Signed-off-by: Rodrigo Vivi rodrigo.vivi@gmail.com

drivers/gpu/drm/i915/i915_reg.h | 16 ++++++++-------- drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++------ 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2521617..7bb3134 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3742,14 +3742,14 @@ #define HSW_VIDEO_DIP_VSC_ECC_B 0x61344 #define HSW_VIDEO_DIP_GCP_B 0x61210

-#define HSW_TVIDEO_DIP_CTL(pipe) \

• _PIPE(pipe, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
  

-#define HSW_TVIDEO_DIP_AVI_DATA(pipe) \

• _PIPE(pipe, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
  

-#define HSW_TVIDEO_DIP_SPD_DATA(pipe) \

• _PIPE(pipe, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
  

-#define HSW_TVIDEO_DIP_GCP(pipe) \

• _PIPE(pipe, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)

+#define HSW_TVIDEO_DIP_CTL(trans) \

• _TRANSCODER(trans, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
  

+#define HSW_TVIDEO_DIP_AVI_DATA(trans) \

• _TRANSCODER(trans, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
  

+#define HSW_TVIDEO_DIP_SPD_DATA(trans) \

• _TRANSCODER(trans, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
  

+#define HSW_TVIDEO_DIP_GCP(trans) \

• _TRANSCODER(trans, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)

Okay, I'm confused. The patch does make sense, but bspec doesn't... the spec says the above HSW_TVIDEO_DIP_* registers (in the 0x6xxxx range) are per pipe, and the TVIDEO_DIP_* registers (in the 0xexxxx range) are per transcoder. Care to enlighten me here...?

Also, outside of this patch, have we reviewed if there's always a register for TRANSCODER_EDP where _TRANSCODER() macro is used?

BR, Jani.

#define _TRANS_HTOTAL_B 0xe1000 #define _TRANS_HBLANK_B 0xe1004 diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index d53b731..ab95e05 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -120,13 +120,13 @@ static u32 hsw_infoframe_enable(struct dip_infoframe *frame) } }

-static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame, enum pipe pipe) +static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame, enum transcoder cpu_transcoder) { switch (frame->type) { case DIP_TYPE_AVI:

• return HSW_TVIDEO_DIP_AVI_DATA(pipe);
  

• return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
  

  case DIP_TYPE_SPD:

• return HSW_TVIDEO_DIP_SPD_DATA(pipe);
  

• return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
  

  default: DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type); return 0;

@@ -293,8 +293,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder, struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);

• u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
• u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->pipe);

• u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
• u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->cpu_transcoder); unsigned int i, len = DIP_HEADER_SIZE + frame->len; u32 val = I915_READ(ctl_reg);

@@ -566,7 +566,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder, struct drm_i915_private *dev_priv = encoder->dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);

• u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);

• u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder); u32 val = I915_READ(reg);

  assert_hdmi_port_disabled(intel_hdmi);

-- 1.7.11.7

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Hi

2013/1/30 Rodrigo Vivi rodrigo.vivi@gmail.com:

While old platforms had 3 transcoders and 3 pipes (1:1), HSW has 4 transcoders and 3 pipes. To avoid future mistakes transcoders must be used here instead of pipes even though it is working right now by coincidence.

Not by coincidence: these regs are currently only used by HDMI code and HDMI code never uses TRANSCODER_EDP, so using "pipe" instead of cpu_transcoder" is correct, since on HDMI pipe is always the same thing as cpu_transcoder. Now that we're going to start to use these registers for DP your patch is needed.

My only complaint about this patch (besides the sentence above) would be due to using more than 80 columns, both on the commit message and coding :)

Reviewed-by: Paulo Zanoni paulo.r.zanoni@intel.com

Signed-off-by: Rodrigo Vivi rodrigo.vivi@gmail.com

drivers/gpu/drm/i915/i915_reg.h | 16 ++++++++-------- drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++------ 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2521617..7bb3134 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3742,14 +3742,14 @@ #define HSW_VIDEO_DIP_VSC_ECC_B 0x61344 #define HSW_VIDEO_DIP_GCP_B 0x61210

-#define HSW_TVIDEO_DIP_CTL(pipe) \

•    _PIPE(pipe, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
  

-#define HSW_TVIDEO_DIP_AVI_DATA(pipe) \

•    _PIPE(pipe, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
  

-#define HSW_TVIDEO_DIP_SPD_DATA(pipe) \

•    _PIPE(pipe, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
  

-#define HSW_TVIDEO_DIP_GCP(pipe) \

•   _PIPE(pipe, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)
  

+#define HSW_TVIDEO_DIP_CTL(trans) \

•    _TRANSCODER(trans, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
  

+#define HSW_TVIDEO_DIP_AVI_DATA(trans) \

•    _TRANSCODER(trans, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
  

+#define HSW_TVIDEO_DIP_SPD_DATA(trans) \

•    _TRANSCODER(trans, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
  

+#define HSW_TVIDEO_DIP_GCP(trans) \

•   _TRANSCODER(trans, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)
  
  

#define _TRANS_HTOTAL_B 0xe1000 #define _TRANS_HBLANK_B 0xe1004 diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index d53b731..ab95e05 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -120,13 +120,13 @@ static u32 hsw_infoframe_enable(struct dip_infoframe *frame) } }

-static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame, enum pipe pipe) +static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame, enum transcoder cpu_transcoder) { switch (frame->type) { case DIP_TYPE_AVI:

•           return HSW_TVIDEO_DIP_AVI_DATA(pipe);
  

•           return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
    case DIP_TYPE_SPD:
  

•           return HSW_TVIDEO_DIP_SPD_DATA(pipe);
  

•           return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
    default:
            DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
            return 0;
  

@@ -293,8 +293,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder, struct drm_device *dev = encoder->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);

•   u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
  

•   u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->pipe);
  

•   u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
  

•   u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->cpu_transcoder);
    unsigned int i, len = DIP_HEADER_SIZE + frame->len;
    u32 val = I915_READ(ctl_reg);
  
  

@@ -566,7 +566,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder, struct drm_i915_private *dev_priv = encoder->dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);

•   u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
  

•   u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
    u32 val = I915_READ(reg);
  
    assert_hdmi_port_disabled(intel_hdmi);
  

-- 1.7.11.7

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx

On Wed, 30 Jan 2013, Rodrigo Vivi rodrigo.vivi@gmail.com wrote:

From: Shobhit Kumar shobhit.kumar@intel.com

Signed-off-by: Sateesh Kavuri sateesh.kavuri@intel.com

v2: Modified and corrected the structures to be more in line for kernel coding guidelines and rebased the code on Paulo's DP patchset

Signed-off-by: Shobhit Kumar shobhit.kumar@intel.com

v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE v4: moving them to include/drm/drm_dp_helper.h and also already icluding EDP_PSR_RECEIVER_CAP_SIZE to add everything needed for PSR at once at drm_dp_helper.h

Signed-off-by: Rodrigo Vivi rodrigo.vivi@gmail.com

include/drm/drm_dp_helper.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index e8e1417..e9b7c4b 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -343,12 +343,41 @@ u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE], int lane);

#define DP_RECEIVER_CAP_SIZE 0xf +#define EDP_PSR_RECEIVER_CAP_SIZE 2

void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]); void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);

u8 drm_dp_link_rate_to_bw_code(int link_rate); int drm_dp_bw_code_to_link_rate(u8 link_bw);

+/* SDP header as per eDP 1.3 spec, section 3.6 */

Section 3.5.

+struct edp_sdp_header {

• u8 id;
• u8 type;
• u8 revision : 5; /* Bits 0:4 */
• u8 rsvd1 : 3; /* Bits 5:7 */
• u8 valid_payload_bytes : 5; /* Bits 0:4 */
• u8 rsvd2 : 3; /* Bits 5:7 */

+} __attribute__((packed));

+/* SDP VSC header as per eDP 1.3 spec, section 3.6 */

Section 3.5.

I really wouldn't mind SDP and VSC being spelled out for the casual reader without eDP spec at hand.

+struct edp_vsc_psr {

• struct edp_sdp_header sdp_header;
• u8 unused;
• u8 psr_state : 1; /* Bit 0 */
• u8 update_rfb : 1; /* Bit 1 */
• u8 valid_crc : 1; /* Bit 2 */
• u8 reserved1 : 5; /* Bits 3:7 */

I don't think the bitfields here are portable for representing the data. A bit of googling suggests GCC lays out the bits as you document above on little-endian machines, but the other way around on big-endian machines. That would be fine within i915 which assumes little-endian for obvious reasons, but not in common drm code.

I think you'll just have to use u8 and #defines for the bits and masks.

BR, Jani.

• u8 crc_r_lower;
• u8 crc_r_higher;
• u8 crc_g_lower;
• u8 crc_g_higher;
• u8 crc_b_lower;
• u8 crc_b_higher;
• u8 reserved2[24];

+} __attribute__((packed));

static inline int drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE]) { -- 1.7.11.7

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi rodrigo.vivi@gmail.com wrote:

From: Shobhit Kumar shobhit.kumar@intel.com

Signed-off-by: Shobhit Kumar shobhit.kumar@intel.com

v2: reuse of just created is_edp_psr and put it at right place.

A couple of nitpicks below, but either way,

Reviewed-by: Jani Nikula jani.nikula@intel.com

Signed-off-by: Rodrigo Vivi rodrigo.vivi@gmail.com

drivers/gpu/drm/i915/intel_dp.c | 12 ++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index cbd3236..aeb0ef1 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1435,6 +1435,11 @@ static void intel_post_disable_dp(struct intel_encoder *encoder) } }

+static bool is_edp_psr(struct intel_dp *intel_dp) +{

• return (is_edp(intel_dp) && (intel_dp->psr_dpcd[0] & 0x1));

s/0x1/DP_PSR_IS_SUPPORTED/

+}

static void intel_enable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); @@ -2111,6 +2116,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] == 0) return false; /* DPCD not present */

• /* Check if the panel supports PSR */
• memset(intel_dp->psr_dpcd, 0, EDP_PSR_RECEIVER_CAP_SIZE);

s/EDP_PSR_RECEIVER_CAP_SIZE/sizeof(intel_dp->psr_dpcd)/

• intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,

• 		       intel_dp->psr_dpcd,
  

• 		       sizeof(intel_dp->psr_dpcd));
  

• if (is_edp_psr(intel_dp))

• DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
  

  if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) return true; /* native DP sink */

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index aeff0d1..d4b3bac 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -371,6 +371,7 @@ struct intel_dp { uint8_t link_bw; uint8_t lane_count; uint8_t dpcd[DP_RECEIVER_CAP_SIZE];

• uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE]; uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; struct i2c_adapter adapter; struct i2c_algo_dp_aux_data algo;

-- 1.7.11.7

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx

On Wed, 30 Jan 2013, Rodrigo Vivi rodrigo.vivi@gmail.com wrote:

From: Shobhit Kumar shobhit.kumar@intel.com

Signed-off-by: Shobhit Kumar shobhit.kumar@intel.com

v2: Created aux_clock_divider function to avoid duplicated code.

Bikeshed, that could be a prep patch, but I won't insist on it.

Other comments below.

Unfortunatelly dp_aux_ch and psr_aux_ch aren't so similar to reuse full code.

Signed-off-by: Rodrigo Vivi rodrigo.vivi@gmail.com

drivers/gpu/drm/i915/i915_reg.h | 13 ++++++ drivers/gpu/drm/i915/intel_dp.c | 89 +++++++++++++++++++++++++++++++--------- drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 7bb3134..10732dc 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1563,6 +1563,19 @@ #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B) #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)

+#define EDP_PSR_AUX_CTL 0x64810 +#define EDP_PSR_AUX_DATA1 0x64814 +#define EDP_PSR_AUX_DATA2 0x64818 +#define EDP_PSR_AUX_DATA3 0x6481c +#define EDP_PSR_AUX_DATA4 0x64820 +#define EDP_PSR_AUX_DATA5 0x64824 +#define EDP_PSR_STATUS_CTL 0x64840 +#define EDP_PSR_STATUS_MASK (7<<29) +#define EDP_PSR_PERF_CNT 0x64844 +#define EDP_PSR_DEBUG_CTL 0x64860 +#define EDP_PSR_DEBUG_MASK_MEMUP (1<<26) +#define EDP_PSR_DEBUG_MASK_HPD (1<<25)

/* VGA port control */ #define ADPA 0x61100 #define PCH_ADPA 0xe1100 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index aeb0ef1..fe83e05 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -364,6 +364,34 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq) return status; }

+static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp) +{

• struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
• struct drm_device *dev = intel_dig_port->base.base.dev;
• struct drm_i915_private *dev_priv = dev->dev_private;
• /* The clock divider is based off the hrawclk,

• * and would like to run at 2MHz. So, take the
  

• * hrawclk value and divide by 2 and use that
  

• *
  

• * Note that PCH attached eDP panels should use a 125MHz input
  

• * clock divider.
  

• */
  

• if (is_cpu_edp(intel_dp)) {

• if (HAS_DDI(dev))
  

• 	return intel_ddi_get_cdclk_freq(dev_priv) >> 1;
  

• else if (IS_VALLEYVIEW(dev))
  

• 	return 100;
  

• else if (IS_GEN6(dev) || IS_GEN7(dev))
  

• 	return 200; /* SNB & IVB eDP input clock at 400Mhz */
  

• else
  

• 	return 225; /* eDP input clock at 450Mhz */
  

• } else if (HAS_PCH_SPLIT(dev))

• return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
  

• else

• return intel_hrawclk(dev) / 2;
  

+}

static int intel_dp_aux_ch(struct intel_dp *intel_dp, uint8_t *send, int send_bytes, @@ -411,26 +439,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, }

intel_dp_check_edp(intel_dp);

• /* The clock divider is based off the hrawclk,

• * and would like to run at 2MHz. So, take the
  

• * hrawclk value and divide by 2 and use that
  

• *
  

• * Note that PCH attached eDP panels should use a 125MHz input
  

• * clock divider.
  

• */
  

• if (is_cpu_edp(intel_dp)) {

• if (HAS_DDI(dev))
  

• 	aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1;
  

• else if (IS_VALLEYVIEW(dev))
  

• 	aux_clock_divider = 100;
  

• else if (IS_GEN6(dev) || IS_GEN7(dev))
  

• 	aux_clock_divider = 200; /* SNB & IVB eDP input clock at 400Mhz */
  

• else
  

• 	aux_clock_divider = 225; /* eDP input clock at 450Mhz */
  

• } else if (HAS_PCH_SPLIT(dev))

• aux_clock_divider = DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
  

• else

• aux_clock_divider = intel_hrawclk(dev) / 2;
  

• aux_clock_divider = get_aux_clock_divider(intel_dp);

  if (IS_GEN6(dev)) precharge = 3;

@@ -1440,6 +1449,46 @@ static bool is_edp_psr(struct intel_dp *intel_dp) return (is_edp(intel_dp) && (intel_dp->psr_dpcd[0] & 0x1)); }

+static void intel_edp_psr_setup(struct intel_dp *intel_dp) +{

• struct drm_device *dev = intel_dp_to_dev(intel_dp);
• struct drm_i915_private *dev_priv = dev->dev_private;
• uint32_t aux_clock_divider;
• int precharge = 0x3;
• int msg_size = 5; /* Header(4) + Message(1) */
• /* No need to setup if already done as these setting are persistent

• * until power states are entered */
  

I find "no need to setup" a not good enough reason to not do it every time. However, the specs say you're not supposed to fiddle with the aux regs while PSR is enabled, which *is* a good reason. So maybe update the comment, so we'll know what we're doing if we decide to change this in the future?

• if (intel_dp->psr_setup)

• return;
  

• /* Setup AUX registers */
• /* Write command on DPCD 0x0600 */
• I915_WRITE(EDP_PSR_AUX_DATA1, 0x80060000);
• /* Set the state to normal operation D0 in DPCD 0x0600 */
• I915_WRITE(EDP_PSR_AUX_DATA2, 0x01000000);

I really really don't like just slamming magic 32-bit values into registers. Copy-pasting from intel_dp_aux_native_write() and intel_dp_aux_ch(), you could do something like this here to clarify a bit:

uint16_t address = DP_SET_POWER; uint8_t msg[] = { AUX_NATIVE_WRITE << 4, address >> 8, address & 0xff, 0, /* len - 1 */ DP_SET_POWER_D0, };

for (i = 0; i < sizeof(msg); i += 4) I915_WRITE(EDP_PSR_AUX_DATA1 + i, pack_aux(msg + i, sizeof(msg) - i));

The above could be cleaned up too, maybe there's common bits to be found with the regular aux write stuff etc. But IMHO with that it's already much easier to figure out, without looking at the specs, what's going on, and you can (almost?) ditch the comments too.

BR, Jani.

• aux_clock_divider = get_aux_clock_divider(intel_dp);
• I915_WRITE(EDP_PSR_AUX_CTL,

•    DP_AUX_CH_CTL_TIME_OUT_400us |
  

•    (msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
  

•    (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
  

•    (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
  

• /* Setup the debug register */
• I915_WRITE(EDP_PSR_DEBUG_CTL, I915_READ(EDP_PSR_DEBUG_CTL) |

• 	EDP_PSR_DEBUG_MASK_MEMUP |
  

• 	EDP_PSR_DEBUG_MASK_HPD);
  

• /* This flag can be made to 0 from pm code so as to reinitialize the

• * AUX register in case of power states, returning from which will not
  

• * maintain the AUX register settings
  

• */
  

• intel_dp->psr_setup = 1;

+}

static void intel_enable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d4b3bac..3fa2dd0 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -383,6 +383,7 @@ struct intel_dp { int backlight_on_delay; int backlight_off_delay; struct delayed_work panel_vdd_work;

• uint8_t psr_setup; bool want_panel_vdd; struct intel_connector *attached_connector;

};

1.7.11.7

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi rodrigo.vivi@gmail.com wrote:

From: Shobhit Kumar shobhit.kumar@intel.com

Added eDP PSR enable functionality. This includes setting the PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config, enabling PSR in the sink via DPCD register and finally enabling PSR on the host. PSR works only in LPSP mode, so put the PIPE_DDI in DDIA always on

This patch is heavily based on initial PSR code by Sateesh Kavuri but is quite different in implementation. Makes use of VBT parsed data and also the code has been cleaned up.

Credits-by: Sateesh Kavuri sateesh.kavuri@intel.com Signed-off-by: Shobhit Kumar shobhit.kumar@intel.com

v2: fix getting base.crtc from intel_dp and fix DDI_EDP_INPUT_A_ON entry v3: Add eDP PSR registers here only when they are really used and use HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder)

Some nitpicks and bikeshedding below, but regardless,

Reviewed-by: Jani Nikula jani.nikula@intel.com

Signed-off-by: Rodrigo Vivi rodrigo.vivi@gmail.com

drivers/gpu/drm/i915/i915_reg.h | 24 ++++++ drivers/gpu/drm/i915/intel_ddi.c | 6 +- drivers/gpu/drm/i915/intel_dp.c | 172 +++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 3 + 4 files changed, 204 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 10732dc..6f87d5b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1563,6 +1563,30 @@ #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B) #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)

+/* HSW eDP PSR registers */ +#define EDP_PSR_CTL 0x64800 +#define EDP_PSR_ENABLE (1<<31) +#define EDP_PSR_LINK_DISABLE (0<<27) +#define EDP_PSR_LINK_STANDBY (1<<27) +#define EDP_PSR_MIN_LINK_ENTRY_TIME_MASK (3<<25) +#define EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES (0<<25) +#define EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES (1<<25) +#define EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES (2<<25) +#define EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES (3<<25) +#define EDP_PSR_MAX_SLEEP_TIME_SHIFT 20 +#define EDP_PSR_SKIP_AUX_EXIT (1<<12) +#define EDP_PSR_TP1_TP2_SEL (0<<11) +#define EDP_PSR_TP1_TP3_SEL (1<<11) +#define EDP_PSR_TP2_TP3_TIME_500us (0<<8) +#define EDP_PSR_TP2_TP3_TIME_100us (1<<8) +#define EDP_PSR_TP2_TP3_TIME_2500us (2<<8) +#define EDP_PSR_TP2_TP3_TIME_0us (3<<8) +#define EDP_PSR_TP1_TIME_500us (0<<4) +#define EDP_PSR_TP1_TIME_100us (1<<4) +#define EDP_PSR_TP1_TIME_2500us (2<<4) +#define EDP_PSR_TP1_TIME_0us (3<<4) +#define EDP_PSR_IDLE_FRAME_SHIFT 0

#define EDP_PSR_AUX_CTL 0x64810 #define EDP_PSR_AUX_DATA1 0x64814 #define EDP_PSR_AUX_DATA2 0x64818 diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 2e904a5..feb7a4a 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -985,9 +985,13 @@ void intel_ddi_enable_pipe_func(struct drm_crtc *crtc) temp |= TRANS_DDI_PHSYNC;

if (cpu_transcoder == TRANSCODER_EDP) {

• struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
  

  switch (pipe) { case PIPE_A:

• 	temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
  

• 	if (intel_dp->psr_dpcd[0] & 0x1)
  

s/0x1/DP_PSR_IS_SUPPORTED/

Ditto elsewhere below.

• 		temp |= TRANS_DDI_EDP_INPUT_A_ON;
  

• 	else
  

• 		temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
  break;
  

  case PIPE_B: temp |= TRANS_DDI_EDP_INPUT_B_ONOFF;

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index fe83e05..d136890 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -83,6 +83,13 @@ static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp) return intel_dig_port->base.base.dev; }

+static struct drm_crtc *intel_dp_to_crtc(struct intel_dp *intel_dp) +{

• struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
• return intel_dig_port->base.base.crtc;

+}

static struct intel_dp *intel_attached_dp(struct drm_connector *connector) { return enc_to_intel_dp(&intel_attached_encoder(connector)->base); @@ -1489,6 +1496,171 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp) intel_dp->psr_setup = 1; }

+static bool +intel_edp_is_psr_enabled(struct intel_dp* intel_dp) +{

• struct drm_device *dev = intel_dp_to_dev(intel_dp);
• struct drm_i915_private *dev_priv = dev->dev_private;
• return (I915_READ(EDP_PSR_CTL) & (1<<31)) ? true : false;

return I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;

+}

+static void +intel_edp_psr_enable_src(struct intel_dp *intel_dp) +{

• struct drm_device *dev = intel_dp_to_dev(intel_dp);
• struct drm_i915_private *dev_priv = dev->dev_private;
• uint32_t max_sleep_time = 0x1f;
• uint32_t val = 0x0;
• /* Use VBT values which are parsed in

• * dev_priv->idle_frames,
  

• * but the BIOS initializes this to zero today
  

• * so hardcode
  

• */
  

• uint32_t idle_frames = 6;
• if (intel_dp->psr_dpcd[1] & 0x1) {

• /* No link training on PSR Exit required */
  

• val |= EDP_PSR_TP2_TP3_TIME_0us;
  

• val |= EDP_PSR_TP1_TIME_0us;
  

• val |= EDP_PSR_SKIP_AUX_EXIT;
  

• } else {

• /* Use these Values from VBT
  

•  * Case values are timings for HSW as of now
  

•  * in multiple of 100us
  

•  */
  

• switch(dev_priv->vbt.psr_wakeup_tp1) {
  

• 	case 1:
  

• 		val |= EDP_PSR_TP1_TIME_100us;
  

• 		break;
  

• 	case 5:
  

• 		val |= EDP_PSR_TP1_TIME_500us;
  

• 		break;
  

• 	case 25:
  

• 		val |= EDP_PSR_TP1_TIME_2500us;
  

• 		break;
  

• 	default:
  

• 		val |= EDP_PSR_TP1_TIME_500us;
  

• 		break;
  

• };
  

• switch(dev_priv->vbt.psr_wakeup_tp2_tp3) {
  

• 	case 1:
  

• 		val |= EDP_PSR_TP2_TP3_TIME_100us;
  

• 		break;
  

• 	case 5:
  

• 		val |= EDP_PSR_TP2_TP3_TIME_500us;
  

• 		break;
  

• 	case 25:
  

• 		val |= EDP_PSR_TP2_TP3_TIME_2500us;
  

• 		break;
  

• 	default:
  

• 		val |= EDP_PSR_TP2_TP3_TIME_500us;
  

• 		break;
  

• };
  

• }
• /* Disable main link. Anyway in HSW steppings today

• * link standby does not work
  

• *
  

• * Later used VBT info (already parsed and available)
  

• * while supporting standby we need to program
  

• * val |= EDP_PSR_MIN_LINK_ENTRY_TIME_X_LINES based on VBT
  

• */
  

• val = (val & ~EDP_PSR_LINK_STANDBY) |

• (max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT) |
  

• (idle_frames << EDP_PSR_IDLE_FRAME_SHIFT) |
  

• EDP_PSR_ENABLE;
  

• I915_WRITE(EDP_PSR_CTL, val);

+}

+void intel_edp_enable_psr(struct intel_dp* intel_dp) +{

• struct drm_device *dev = intel_dp_to_dev(intel_dp);
• struct drm_i915_private *dev_priv = dev->dev_private;
• struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
• struct edp_vsc_psr psr_vsc;
• uint32_t reg = HSW_TVIDEO_DIP_AVI_DATA(intel_crtc->cpu_transcoder);
• uint32_t *vsc_data = (uint32_t *) &psr_vsc;
• int i = 0, vsc_len = sizeof(struct edp_vsc_psr);
• if (!is_edp_psr(intel_dp))

• return;
  

• /* setup AUX registers in case returned from pm states */
• intel_edp_psr_setup(intel_dp);
• /* Check if PSR is already enabled */
• if (!intel_edp_is_psr_enabled(intel_dp)) {

I'd rather make this return early if PSR is enabled, and reduce indent on the lines below.

• /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
  

• memset(&psr_vsc, 0, sizeof(psr_vsc));
  

• psr_vsc.sdp_header.id = 0;
  

• psr_vsc.sdp_header.type = 0x7;
  

• psr_vsc.sdp_header.revision = 0x2;
  

• psr_vsc.sdp_header.valid_payload_bytes = 0x8;
  

• /* As per eDP spec, wait for vblank to send SDP VSC packet */
  

• intel_wait_for_vblank(dev, intel_crtc->pipe);
  

• /* Load the VSC DIP packet */
  

• for(i = 0; i < vsc_len; i += 4)
  

Just use sizeof here too instead of a temp variable for vsc_len.

• 	I915_WRITE((reg + i), vsc_data[i]);
  

Unnecessary braces around reg + i.

I guess we can be sure sizeof(struct edp_vsc_psr) % 4 == 0 always.

+#if 0

• /* TBD:
  

•  * We might not have to do explicitely as hardware will take care of this */
  

• /* Enable the DIP register */
  

• val = I915_READ(VIDEO_DIP_CTL_EDP);
  

• I915_WRITE(VIDEO_DIP_CTL_EDP, val | VIDEOP_DIP_VSC);
  

+#endif

• /* Enable PSR in sink by setting bit 0 in DPCD config reg
  

•  * along with the transmitter state during PSR active
  

•  * Transmitter state later can be ready from VBT. As of now
  

•  * program the full link down
  

•  *
  

•  */
  

• intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
  

• 				DP_PSR_ENABLE &
  

• 				~DP_PSR_MAIN_LINK_ACTIVE);
  

• /* Enable PSR on the host */
  

• intel_edp_psr_enable_src(intel_dp);
  

• }

+}

+void intel_edp_disable_psr(struct intel_dp* intel_dp) +{

• struct drm_device *dev = intel_dp_to_dev(intel_dp);
• struct drm_i915_private *dev_priv = dev->dev_private;
• struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
• uint32_t val;
• if (!intel_edp_is_psr_enabled(intel_dp))

• return;
  

• val = I915_READ(EDP_PSR_CTL);
• I915_WRITE(EDP_PSR_CTL, (val & ~EDP_PSR_ENABLE));

Unnecessary braces.

• /* Wait till PSR is idle */
• if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) & EDP_PSR_STATUS_MASK) == 0, 2000, 10))

• DRM_ERROR("Timed out waiting for PSR Idle State\n");
  

• intel_wait_for_vblank(dev, intel_crtc->pipe);

+#if 0

• /* TBD:

• * Following is not yet confirmed from H/W team.
  

• * As per last discussion we do not need to disable
  

• * VSC DIP explicitely. Just maintaining the code in
  

• * case we have to do this later at some point
  

• */
  

• /* Disable VSC DIP */
• val = I915_READ(VIDEO_DIP_CTL_EDP);
• I915_WRITE(VIDEO_DIP_CTL_EDP, val & ~VIDEOP_DIP_VSC);

+#endif +}

static void intel_enable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 3fa2dd0..2f48e5c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -692,4 +692,7 @@ extern bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector); extern void intel_ddi_fdi_disable(struct drm_crtc *crtc);

+extern void intel_edp_enable_psr(struct intel_dp* intel_dp); +extern void intel_edp_disable_psr(struct intel_dp* intel_dp);

#endif /* __INTEL_DRV_H__ */

1.7.11.7

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel