[Intel-gfx] [PATCH v5 1/4] drm/i915: Add audio sync_audio_rate callback

[libin . yang]

From: Libin Yang 

Add the sync_audio_rate callback.

With the callback, audio driver can trigger
i915 driver to set the proper N/CTS or N/M
based on different sample rates.

Signed-off-by: Libin Yang 
---
 include/drm/i915_component.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index c9a8b64..aabebcb 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -33,6 +33,7 @@ struct i915_audio_component {
void (*put_power)(struct device *);
void (*codec_wake_override)(struct device *, bool enable);
int (*get_cdclk_freq)(struct device *);
+   int (*sync_audio_rate)(struct device *, int port, int rate);
} *ops;
 };
 
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback

[libin . yang]

From: Libin Yang 

HDMI audio may not work at some frequencies
with the HW provided N/CTS.

This patch sets the proper N value for the
given audio sample rate at the impacted frequencies.
At other frequencies, it will use the N/CTS value
which HW provides.

Signed-off-by: Libin Yang 
---
 drivers/gpu/drm/i915/intel_audio.c | 119 +
 1 file changed, 119 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index dc32cf4..96b97be 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -68,6 +68,31 @@ static const struct {
{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
 };
 
+/* HDMI N/CTS table */
+#define TMDS_297M 297000
+#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
+static const struct {
+   int sample_rate;
+   int clock;
+   int n;
+   int cts;
+} aud_ncts[] = {
+   { 44100, TMDS_296M, 4459, 234375 },
+   { 44100, TMDS_297M, 4704, 247500 },
+   { 48000, TMDS_296M, 5824, 281250 },
+   { 48000, TMDS_297M, 5120, 247500 },
+   { 32000, TMDS_296M, 5824, 421875 },
+   { 32000, TMDS_297M, 3072, 222750 },
+   { 88200, TMDS_296M, 8918, 234375 },
+   { 88200, TMDS_297M, 9408, 247500 },
+   { 96000, TMDS_296M, 11648, 281250 },
+   { 96000, TMDS_297M, 10240, 247500 },
+   { 176400, TMDS_296M, 17836, 234375 },
+   { 176400, TMDS_297M, 18816, 247500 },
+   { 44100, TMDS_296M, 23296, 281250 },
+   { 44100, TMDS_297M, 20480, 247500 },
+};
+
 /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
 static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
 {
@@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct 
drm_display_mode *mode)
return hdmi_audio_clock[i].config;
 }
 
+static int audio_config_get_n(struct drm_display_mode *mode, int rate)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
+   if ((rate == aud_ncts[i].sample_rate) &&
+   (mode->clock == aud_ncts[i].clock)) {
+   return aud_ncts[i].n;
+   }
+   }
+   return 0;
+}
+
+/* check whether N/CTS/M need be set manually */
+static bool audio_rate_need_prog(struct intel_crtc *crtc,
+   struct drm_display_mode *mode)
+{
+   if (((mode->clock == TMDS_297M) ||
+(mode->clock == TMDS_296M)) &&
+   intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
+   return true;
+   else
+   return false;
+}
+
 static bool intel_eld_uptodate(struct drm_connector *connector,
   int reg_eldv, uint32_t bits_eldv,
   int reg_elda, uint32_t bits_elda,
@@ -514,12 +564,81 @@ static int i915_audio_component_get_cdclk_freq(struct 
device *dev)
return ret;
 }
 
+static int i915_audio_component_sync_audio_rate(struct device *dev,
+   int port, int rate)
+{
+   struct drm_i915_private *dev_priv = dev_to_i915(dev);
+   struct drm_device *drm_dev = dev_priv->dev;
+   struct intel_encoder *intel_encoder;
+   struct intel_digital_port *intel_dig_port;
+   struct intel_crtc *crtc;
+   struct drm_display_mode *mode;
+   enum pipe pipe = -1;
+   u32 tmp;
+   int n_low, n_up, n;
+
+   /* 1. get the pipe */
+   for_each_intel_encoder(drm_dev, intel_encoder) {
+   if (intel_encoder->type != INTEL_OUTPUT_HDMI)
+   continue;
+   intel_dig_port = enc_to_dig_port(&intel_encoder->base);
+   if (port == intel_dig_port->port) {
+   crtc = to_intel_crtc(intel_encoder->base.crtc);
+   if (!crtc) {
+   DRM_DEBUG_KMS("%s: crtc is NULL\n", __func__);
+   continue;
+   }
+   pipe = crtc->pipe;
+   break;
+   }
+   }
+
+   if (pipe == INVALID_PIPE) {
+   DRM_DEBUG_KMS("no pipe for the port %c\n", port_name(port));
+   return -ENODEV;
+   }
+   DRM_DEBUG_KMS("pipe %c connects port %c\n",
+ pipe_name(pipe), port_name(port));
+   mode = &crtc->config->base.adjusted_mode;
+
+   /* 2. check whether to set the N/CTS/M manually or not */
+   if (!audio_rate_need_prog(crtc, mode)) {
+   tmp = I915_READ(HSW_AUD_CFG(pipe));
+   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+   return 0;
+   }
+
+   n = audio_config_get_n(mode, rate);
+   if (n == 0) {
+   DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
+ port_name(port));
+   tmp = I915_READ(HSW_AUD_CFG(pipe));
+   tmp &= ~AUD_C

[Intel-gfx] [PATCH v5 3/4] ALSA: hda - display audio call sync_audio_rate callback

[libin . yang]

From: Libin Yang 

For display audio, call the sync_audio_rate callback function
to do the synchronization between gfx driver and audio driver.

Signed-off-by: Libin Yang 
Reviewed-by: Takashi Iwai 
---
 sound/pci/hda/patch_hdmi.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index a97db5f..1668868 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1770,6 +1770,16 @@ static bool check_non_pcm_per_cvt(struct hda_codec 
*codec, hda_nid_t cvt_nid)
return non_pcm;
 }
 
+/* There is a fixed mapping between audio pin node and display port
+ * on current Intel platforms:
+ * Pin Widget 5 - PORT B (port = 1 in i915 driver)
+ * Pin Widget 6 - PORT C (port = 2 in i915 driver)
+ * Pin Widget 7 - PORT D (port = 3 in i915 driver)
+ */
+static int intel_pin2port(hda_nid_t pin_nid)
+{
+   return pin_nid - 4;
+}
 
 /*
  * HDMI callbacks
@@ -1786,6 +1796,8 @@ static int generic_hdmi_playback_pcm_prepare(struct 
hda_pcm_stream *hinfo,
int pin_idx = hinfo_to_pin_index(codec, hinfo);
struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
hda_nid_t pin_nid = per_pin->pin_nid;
+   struct snd_pcm_runtime *runtime = substream->runtime;
+   struct i915_audio_component *acomp = codec->bus->core.audio_component;
bool non_pcm;
int pinctl;
 
@@ -1802,6 +1814,13 @@ static int generic_hdmi_playback_pcm_prepare(struct 
hda_pcm_stream *hinfo,
intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx);
}
 
+   /* Call sync_audio_rate to set the N/CTS/M manually if necessary */
+   /* Todo: add DP1.2 MST audio support later */
+   if (acomp && acomp->ops && acomp->ops->sync_audio_rate)
+   acomp->ops->sync_audio_rate(acomp->dev,
+   intel_pin2port(pin_nid),
+   runtime->rate);
+
non_pcm = check_non_pcm_per_cvt(codec, cvt_nid);
mutex_lock(&per_pin->lock);
per_pin->channels = substream->runtime->channels;
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 4/4] drm/i915: set proper N/CTS in modeset

[libin . yang]

From: Libin Yang 

When modeset occurs and the TMDS frequency is set to some
speical values, the N/CTS need to be set manually if audio
is playing.

Signed-off-by: Libin Yang 
---
 drivers/gpu/drm/i915/i915_reg.h|  8 
 drivers/gpu/drm/i915/intel_audio.c | 40 +-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6786e94..122b5bd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7035,6 +7035,8 @@ enum skl_disp_power_wells {
_HSW_AUD_MISC_CTRL_A, \
_HSW_AUD_MISC_CTRL_B)
 
+#define HSW_AUD_PIPE_CONN_SEL_CTRL  0x650ac
+
 #define _HSW_AUD_DIP_ELD_CTRL_ST_A 0x650b4
 #define _HSW_AUD_DIP_ELD_CTRL_ST_B 0x651b4
 #define HSW_AUD_DIP_ELD_CTRL(pipe) _PIPE(pipe, \
@@ -7049,6 +7051,12 @@ enum skl_disp_power_wells {
_HSW_AUD_DIG_CNVT_2)
 #define DIP_PORT_SEL_MASK  0x3
 
+#define _HSW_AUD_STR_DESC_10x65084
+#define _HSW_AUD_STR_DESC_20x65184
+#define AUD_STR_DESC(pipe) _PIPE(pipe, \
+_HSW_AUD_STR_DESC_1,   \
+_HSW_AUD_STR_DESC_2)
+
 #define _HSW_AUD_EDID_DATA_A   0x65050
 #define _HSW_AUD_EDID_DATA_B   0x65150
 #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 96b97be..0dfdc77 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -140,6 +140,27 @@ static bool audio_rate_need_prog(struct intel_crtc *crtc,
return false;
 }
 
+static int audio_config_get_rate(struct drm_i915_private *dev_priv,
+   enum pipe pipe)
+{
+   uint32_t tmp;
+   int cvt_idx;
+   int base_rate, mul, div, rate;
+
+   tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL);
+   cvt_idx = (tmp >> (pipe * 8)) & 0xff;
+   tmp = I915_READ(AUD_STR_DESC(cvt_idx));
+   base_rate = tmp & (1 << 14);
+   if (base_rate == 0)
+   rate = 48000;
+   else
+   rate = 44100;
+   mul = (tmp & (0x7 << 11)) + 1;
+   div = (tmp & (0x7 << 8)) + 1;
+   rate = rate * mul / div;
+   return rate;
+}
+
 static bool intel_eld_uptodate(struct drm_connector *connector,
   int reg_eldv, uint32_t bits_eldv,
   int reg_elda, uint32_t bits_elda,
@@ -261,6 +282,8 @@ static void hsw_audio_codec_enable(struct drm_connector 
*connector,
const uint8_t *eld = connector->eld;
uint32_t tmp;
int len, i;
+   int n_low, n_up, n;
+   int rate;
 
DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
  pipe_name(pipe), drm_eld_size(eld));
@@ -296,12 +319,27 @@ static void hsw_audio_codec_enable(struct drm_connector 
*connector,
/* Enable timestamps */
tmp = I915_READ(HSW_AUD_CFG(pipe));
tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
-   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
tmp |= AUD_CONFIG_N_VALUE_INDEX;
else
tmp |= audio_config_hdmi_pixel_clock(mode);
+
+   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+   if (audio_rate_need_prog(intel_crtc, mode)) {
+   rate = audio_config_get_rate(dev_priv, pipe);
+   n = audio_config_get_n(mode, rate);
+   if (n != 0) {
+   n_low = n & 0xfff;
+   n_up = (n >> 12) & 0xff;
+   tmp &= ~(AUD_CONFIG_UPPER_N_MASK |
+AUD_CONFIG_LOWER_N_MASK);
+   tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
+   (n_low << AUD_CONFIG_LOWER_N_SHIFT) |
+   AUD_CONFIG_N_PROG_ENABLE);
+   }
+   }
+
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 }
 
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Avoid TP3 on CHV

[Jani Nikula]

On Tue, 18 Aug 2015, Sivakumar Thulasimani  
wrote:
> On 8/18/2015 12:14 PM, Jani Nikula wrote:
>> On Tue, 18 Aug 2015, Sivakumar Thulasimani  
>> wrote:
>>> From: "Thulasimani,Sivakumar" 
>>>
>>> This patch removes TP3 support on CHV since there is no support
>>> for HBR2 on this platform.
>>>
>>> v2: rename the function to indicate it checks source rates (Jani)
>>>
>>> Reviewed-by: Ville Syrjälä 
>>> Signed-off-by: Sivakumar Thulasimani 
>>> ---
>>>   drivers/gpu/drm/i915/intel_dp.c |   24 +---
>>>   1 file changed, 17 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>>> b/drivers/gpu/drm/i915/intel_dp.c
>>> index 475d8cb..8bc6361 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -1207,6 +1207,20 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const 
>>> int **sink_rates)
>>> return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
>>>   }
>>>   
>>> +static bool intel_dp_source_supports_hbr2(struct drm_device *dev)
>>> +{
>>> +   /* WaDisableHBR2:skl */
>>> +   if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>>> +   return false;
>>> +
>>> +   if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
>>> +   (INTEL_INFO(dev)->gen >= 9))
>>> +   return true;
>>> +   else
>>> +   return false;
>>> +}
>>> +
>>> +
>>>   static int
>>>   intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>>>   {
>>> @@ -1220,12 +1234,8 @@ intel_dp_source_rates(struct drm_device *dev, const 
>>> int **source_rates)
>>>   
>>> *source_rates = default_rates;
>>>   
>>> -   /* WaDisableHBR2:skl */
>>> -   if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>>> -   return (DP_LINK_BW_2_7 >> 3) + 1;
>>> -
>>> -   if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
>>> -   (INTEL_INFO(dev)->gen >= 9))
>>> +   /* This depends on the fact that 5.4 is last value in the array */
>>> +   if (intel_dp_source_supports_hbr2(dev))
>>> return (DP_LINK_BW_5_4 >> 3) + 1;
>>> else
>>> return (DP_LINK_BW_2_7 >> 3) + 1;
>>> @@ -3926,7 +3936,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>>> /* Training Pattern 3 support, both source and sink */
>>> if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
>>> intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
>>> -   (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)) {
>>> +   intel_dp_source_supports_hbr2(dev)) {
>> hbr2 is not the same as tps3, is it? It's possible to use tps3 without
>> using hbr2, right?
>>
>> BR,
>> Jani.
> Yes, TP3 can be supported on panels that does not support HBR2 as well, but
> the check here is for hardware capability. Intel platforms that does not 
> support
> HBR2 cannot support TP3 as well, so we should treat them both the same.
> (the only exception here seems to be SKL 
>>
>>> intel_dp->use_tps3 = true;
>>> DRM_DEBUG_KMS("Displayport TPS3 supported\n");
>>> } else
>>> -- 
>>> 1.7.9.5
>>>
>
> -- 
> regards,
> Sivakumar
>

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH for v4.2] Revert "drm/i915: Allow parsing of variable size child device entries from VBT"

[Jani Nikula]

This reverts

commit 047fe6e6db9161e69271f56daaafdaf2add023b1
Author: David Weinehall 
Date:   Tue Aug 4 16:55:52 2015 +0300

drm/i915: Allow parsing of variable size child device entries from VBT

That commit is not valid for v4.2, however it will be valid for v4.3. It
was simply queued too early.

The referenced regressing commit is just fine until the size of struct
common_child_dev_config changes, and that won't happen until
v4.3. Indeed, the expected size checks here rely on the increased size
of the struct, breaking new platforms.

Fixes: 047fe6e6db91 ("drm/i915: Allow parsing of variable size child device 
entries from VBT")
Cc: Daniel Vetter 
Cc: David Weinehall 
Cc: Ville Syrjälä 
Signed-off-by: Jani Nikula 

---

There was another patch from David increasing the size of the struct
[1], but that then regresses sdvo mapping parsing. It's the simplest to
just revert and fix this up properly for v4.3.

[1] http://mid.gmane.org/20150813131415.GO6150@boom
---
 drivers/gpu/drm/i915/intel_bios.c | 27 ---
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 3dcd59e694db..198fc3c3291b 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1075,34 +1075,15 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
const union child_device_config *p_child;
union child_device_config *child_dev_ptr;
int i, child_device_num, count;
-   u8 expected_size;
-   u16 block_size;
+   u16 block_size;
 
p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
if (!p_defs) {
DRM_DEBUG_KMS("No general definition block is found, no devices 
defined.\n");
return;
}
-   if (bdb->version < 195) {
-   expected_size = 33;
-   } else if (bdb->version == 195) {
-   expected_size = 37;
-   } else if (bdb->version <= 197) {
-   expected_size = 38;
-   } else {
-   expected_size = 38;
-   DRM_DEBUG_DRIVER("Expected child_device_config size for BDB 
version %u not known; assuming %u\n",
-expected_size, bdb->version);
-   }
-
-   if (expected_size > sizeof(*p_child)) {
-   DRM_ERROR("child_device_config cannot fit in p_child\n");
-   return;
-   }
-
-   if (p_defs->child_dev_size != expected_size) {
-   DRM_ERROR("Size mismatch; child_device_config size=%u (expected 
%u); bdb->version: %u\n",
- p_defs->child_dev_size, expected_size, bdb->version);
+   if (p_defs->child_dev_size < sizeof(*p_child)) {
+   DRM_ERROR("General definiton block child device size is too 
small.\n");
return;
}
/* get the block size of general definitions */
@@ -1149,7 +1130,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 
child_dev_ptr = dev_priv->vbt.child_dev + count;
count++;
-   memcpy(child_dev_ptr, p_child, p_defs->child_dev_size);
+   memcpy(child_dev_ptr, p_child, sizeof(*p_child));
}
return;
 }
-- 
2.1.4

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

[Intel-gfx] [PATCH] drm/i915: Avoid TP3 on CHV

[Sivakumar Thulasimani]

From: "Thulasimani,Sivakumar" 

This patch removes TP3 support on CHV since there is no support
for HBR2 on this platform.

v2: rename the function to indicate it checks source rates (Jani)
v3: update comment to indicate TP3 dependancy on HBR2 supported
hardware (Jani)

Reviewed-by: Ville Syrjälä 
Signed-off-by: Sivakumar Thulasimani 
---
 drivers/gpu/drm/i915/intel_dp.c |   31 +++
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 475d8cb..051614a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1207,6 +1207,20 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int 
**sink_rates)
return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
 }
 
+static bool intel_dp_source_supports_hbr2(struct drm_device *dev)
+{
+   /* WaDisableHBR2:skl */
+   if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
+   return false;
+
+   if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
+   (INTEL_INFO(dev)->gen >= 9))
+   return true;
+   else
+   return false;
+}
+
+
 static int
 intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
 {
@@ -1220,12 +1234,8 @@ intel_dp_source_rates(struct drm_device *dev, const int 
**source_rates)
 
*source_rates = default_rates;
 
-   /* WaDisableHBR2:skl */
-   if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
-   return (DP_LINK_BW_2_7 >> 3) + 1;
-
-   if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
-   (INTEL_INFO(dev)->gen >= 9))
+   /* This depends on the fact that 5.4 is last value in the array */
+   if (intel_dp_source_supports_hbr2(dev))
return (DP_LINK_BW_5_4 >> 3) + 1;
else
return (DP_LINK_BW_2_7 >> 3) + 1;
@@ -3923,10 +3933,15 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
}
}
 
-   /* Training Pattern 3 support, both source and sink */
+   /* Training Pattern 3 support, Intel platforms that support HBR2 alone
+* have support for TP3 hence that check is used along with dpcd check
+* to ensure TP3 can be enabled.
+* SKL < B0: due it's WaDisableHBR2 is the only exception where TP3 is
+* supported but still not enabled.
+*/
if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
-   (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)) {
+   intel_dp_source_supports_hbr2(dev)) {
intel_dp->use_tps3 = true;
DRM_DEBUG_KMS("Displayport TPS3 supported\n");
} else
-- 
1.7.9.5

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

Re: [Intel-gfx] [PATCH for v4.2] Revert "drm/i915: Allow parsing of variable size child device entries from VBT"

[Ville Syrjälä]

On Tue, Aug 18, 2015 at 12:33:36PM +0300, Jani Nikula wrote:
> This reverts
> 
> commit 047fe6e6db9161e69271f56daaafdaf2add023b1
> Author: David Weinehall 
> Date:   Tue Aug 4 16:55:52 2015 +0300
> 
> drm/i915: Allow parsing of variable size child device entries from VBT
> 
> That commit is not valid for v4.2, however it will be valid for v4.3. It
> was simply queued too early.
> 
> The referenced regressing commit is just fine until the size of struct
> common_child_dev_config changes, and that won't happen until
> v4.3. Indeed, the expected size checks here rely on the increased size
> of the struct, breaking new platforms.
> 
> Fixes: 047fe6e6db91 ("drm/i915: Allow parsing of variable size child device 
> entries from VBT")
> Cc: Daniel Vetter 
> Cc: David Weinehall 
> Cc: Ville Syrjälä 
> Signed-off-by: Jani Nikula 

Reviewed-by: Ville Syrjälä 

> 
> ---
> 
> There was another patch from David increasing the size of the struct
> [1], but that then regresses sdvo mapping parsing. It's the simplest to
> just revert and fix this up properly for v4.3.
> 
> [1] http://mid.gmane.org/20150813131415.GO6150@boom
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 27 ---
>  1 file changed, 4 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 3dcd59e694db..198fc3c3291b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1075,34 +1075,15 @@ parse_device_mapping(struct drm_i915_private 
> *dev_priv,
>   const union child_device_config *p_child;
>   union child_device_config *child_dev_ptr;
>   int i, child_device_num, count;
> - u8 expected_size;
> - u16 block_size;
> + u16 block_size;
>  
>   p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
>   if (!p_defs) {
>   DRM_DEBUG_KMS("No general definition block is found, no devices 
> defined.\n");
>   return;
>   }
> - if (bdb->version < 195) {
> - expected_size = 33;
> - } else if (bdb->version == 195) {
> - expected_size = 37;
> - } else if (bdb->version <= 197) {
> - expected_size = 38;
> - } else {
> - expected_size = 38;
> - DRM_DEBUG_DRIVER("Expected child_device_config size for BDB 
> version %u not known; assuming %u\n",
> -  expected_size, bdb->version);
> - }
> -
> - if (expected_size > sizeof(*p_child)) {
> - DRM_ERROR("child_device_config cannot fit in p_child\n");
> - return;
> - }
> -
> - if (p_defs->child_dev_size != expected_size) {
> - DRM_ERROR("Size mismatch; child_device_config size=%u (expected 
> %u); bdb->version: %u\n",
> -   p_defs->child_dev_size, expected_size, bdb->version);
> + if (p_defs->child_dev_size < sizeof(*p_child)) {
> + DRM_ERROR("General definiton block child device size is too 
> small.\n");
>   return;
>   }
>   /* get the block size of general definitions */
> @@ -1149,7 +1130,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>  
>   child_dev_ptr = dev_priv->vbt.child_dev + count;
>   count++;
> - memcpy(child_dev_ptr, p_child, p_defs->child_dev_size);
> + memcpy(child_dev_ptr, p_child, sizeof(*p_child));
>   }
>   return;
>  }
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping

[Jani Nikula]

commit 75067ddecf21271631bc018d2fb23ddd09b66aae
Author: Antti Koskipaa 
Date:   Fri Jul 10 14:10:55 2015 +0300

drm/i915: Per-DDI I_boost override

increased size of union child_device_config without taking into account
the size check in parse_sdvo_device_mapping(). Switch the function over
to using the legacy struct only.

Fixes: 75067ddecf21 ("drm/i915: Per-DDI I_boost override")
Cc: Antti Koskipaa 
Cc: David Weinehall 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_bios.c | 50 +++
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 8e46149bafdd..1b7e1a591a37 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -401,7 +401,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
 {
struct sdvo_device_mapping *p_mapping;
const struct bdb_general_definitions *p_defs;
-   const union child_device_config *p_child;
+   const struct old_child_dev_config *child; /* legacy */
int i, child_device_num, count;
u16 block_size;
 
@@ -410,14 +410,14 @@ parse_sdvo_device_mapping(struct drm_i915_private 
*dev_priv,
DRM_DEBUG_KMS("No general definition block is found, unable to 
construct sdvo mapping.\n");
return;
}
-   /* judge whether the size of child device meets the requirements.
-* If the child device size obtained from general definition block
-* is different with sizeof(struct child_device_config), skip the
-* parsing of sdvo device info
+
+   /*
+* Only parse SDVO mappings when the general definitions block child
+* device size matches that of the *legacy* child device config
+* struct. Thus, SDVO mapping will be skipped for newer VBT.
 */
-   if (p_defs->child_dev_size != sizeof(*p_child)) {
-   /* different child dev size . Ignore it */
-   DRM_DEBUG_KMS("different child size is found. Invalid.\n");
+   if (p_defs->child_dev_size != sizeof(*child)) {
+   DRM_DEBUG_KMS("Unsupported child device size for SDVO 
mapping.\n");
return;
}
/* get the block size of general definitions */
@@ -427,37 +427,37 @@ parse_sdvo_device_mapping(struct drm_i915_private 
*dev_priv,
p_defs->child_dev_size;
count = 0;
for (i = 0; i < child_device_num; i++) {
-   p_child = child_device_ptr(p_defs, i);
-   if (!p_child->old.device_type) {
+   child = &child_device_ptr(p_defs, i)->old;
+   if (!child->device_type) {
/* skip the device block if device type is invalid */
continue;
}
-   if (p_child->old.slave_addr != SLAVE_ADDR1 &&
-   p_child->old.slave_addr != SLAVE_ADDR2) {
+   if (child->slave_addr != SLAVE_ADDR1 &&
+   child->slave_addr != SLAVE_ADDR2) {
/*
 * If the slave address is neither 0x70 nor 0x72,
 * it is not a SDVO device. Skip it.
 */
continue;
}
-   if (p_child->old.dvo_port != DEVICE_PORT_DVOB &&
-   p_child->old.dvo_port != DEVICE_PORT_DVOC) {
+   if (child->dvo_port != DEVICE_PORT_DVOB &&
+   child->dvo_port != DEVICE_PORT_DVOC) {
/* skip the incorrect SDVO port */
DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
continue;
}
DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
-   " %s port\n",
-   p_child->old.slave_addr,
-   (p_child->old.dvo_port == DEVICE_PORT_DVOB) ?
-   "SDVOB" : "SDVOC");
-   p_mapping = &(dev_priv->sdvo_mappings[p_child->old.dvo_port - 
1]);
+ " %s port\n",
+ child->slave_addr,
+ (child->dvo_port == DEVICE_PORT_DVOB) ?
+ "SDVOB" : "SDVOC");
+   p_mapping = &(dev_priv->sdvo_mappings[child->dvo_port - 1]);
if (!p_mapping->initialized) {
-   p_mapping->dvo_port = p_child->old.dvo_port;
-   p_mapping->slave_addr = p_child->old.slave_addr;
-   p_mapping->dvo_wiring = p_child->old.dvo_wiring;
-   p_mapping->ddc_pin = p_child->old.ddc_pin;
-   p_mapping->i2c_pin = p_child->old.i2c_pin;
+   p_mapping->dvo_port = child->dvo_port;
+   p_mapping->slave_addr = child->slave_addr;
+ 

[Intel-gfx] [PATCH v5 3/4] drm/i915: DSI pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to DSI.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- max_pixclk variable renamed as max_dotclk
- moved dot clock checking inside 'if (fixed_mode)'

V5:
- dot clock checked against fixed_mode clock

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_dsi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 4a601cf..781c267 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -654,6 +654,7 @@ intel_dsi_mode_valid(struct drm_connector *connector,
 {
struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
+   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
 
DRM_DEBUG_KMS("\n");
 
@@ -667,6 +668,8 @@ intel_dsi_mode_valid(struct drm_connector *connector,
return MODE_PANEL;
if (mode->vdisplay > fixed_mode->vdisplay)
return MODE_PANEL;
+   if (fixed_mode->clock > max_dotclk)
+   return MODE_CLOCK_HIGH;
}
 
return MODE_OK;
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 2/4] drm/i915: LVDS pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to LVDS.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- moved supported dotclock check from mode_valid() to intel_lvds_init()

V5:
- dotclock check moved back to mode_valid() function
- dotclock check for fixed mode

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_lvds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
b/drivers/gpu/drm/i915/intel_lvds.c
index 881b5d1..0794dc8 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -289,11 +289,14 @@ intel_lvds_mode_valid(struct drm_connector *connector,
 {
struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
+   int max_pixclk = to_i915(connector->dev)->max_dotclk_freq;
 
if (mode->hdisplay > fixed_mode->hdisplay)
return MODE_PANEL;
if (mode->vdisplay > fixed_mode->vdisplay)
return MODE_PANEL;
+   if (fixed_mode->clock > max_pixclk)
+   return MODE_CLOCK_HIGH;
 
return MODE_OK;
 }
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 1/4] drm/i915: Store max dotclock

[Mika Kahola]

Store max dotclock into dev_priv structure so we are able
to filter out the modes that are not supported by our
platforms.

V2:
- limit the max dot clock frequency to max CD clock frequency
  for the gen9 and above
- limit the max dot clock frequency to 90% of the max CD clock
  frequency for the older gens
- for Cherryview the max dot clock frequency is limited to 95%
  of the max CD clock frequency
- for gen2 and gen3 the max dot clock limit is set to 90% of the
  2X max CD clock frequency

V3:
- max_dotclk variable renamed as max_dotclk_freq in i915_drv.h
- in intel_compute_max_dotclk() the rounding method changed from
  round up to round down when computing max dotclock

V4:
- Haswell and Broadwell supports now dot clocks up to max CD clock
  frequency

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 20 
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e0f3f05..4696685 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1792,6 +1792,7 @@ struct drm_i915_private {
unsigned int fsb_freq, mem_freq, is_ddr3;
unsigned int skl_boot_cdclk;
unsigned int cdclk_freq, max_cdclk_freq;
+   unsigned int max_dotclk_freq;
unsigned int hpll_freq;
 
/**
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f604ce1..a0f790d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5271,6 +5271,21 @@ static void modeset_update_crtc_power_domains(struct 
drm_atomic_state *state)
modeset_put_power_domains(dev_priv, put_domains[i]);
 }
 
+static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
+{
+   int max_cdclk_freq = dev_priv->max_cdclk_freq;
+
+   if (INTEL_INFO(dev_priv)->gen >= 9 ||
+   IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+   return max_cdclk_freq;
+   else if (IS_CHERRYVIEW(dev_priv))
+   return max_cdclk_freq*95/100;
+   else if (INTEL_INFO(dev_priv)->gen < 4)
+   return 2*max_cdclk_freq*90/100;
+   else
+   return max_cdclk_freq*90/100;
+}
+
 static void intel_update_max_cdclk(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5310,8 +5325,13 @@ static void intel_update_max_cdclk(struct drm_device 
*dev)
dev_priv->max_cdclk_freq = dev_priv->cdclk_freq;
}
 
+   dev_priv->max_dotclk_freq = intel_compute_max_dotclk(dev_priv);
+
DRM_DEBUG_DRIVER("Max CD clock rate: %d kHz\n",
 dev_priv->max_cdclk_freq);
+
+   DRM_DEBUG_DRIVER("Max dotclock rate: %d kHz\n",
+dev_priv->max_dotclk_freq);
 }
 
 static void intel_update_cdclk(struct drm_device *dev)
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 4/4] drm/i915: DVO pixel clock check

[Mika Kahola]

It is possible the we request to have a mode that has
higher pixel clock than our HW can support. This patch
checks if requested pixel clock is lower than the one
supported by the HW. The requested mode is discarded
if we cannot support the requested pixel clock.

This patch applies to DVO.

V2:
- removed computation for max pixel clock

V3:
- cleanup by removing unnecessary lines

V4:
- clock check against max dotclock moved inside 'if (fixed_mode)'

V5:
- dot clock check against fixed_mode clock when available

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_dvo.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index dc532bb..c80fe1f 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -201,6 +201,8 @@ intel_dvo_mode_valid(struct drm_connector *connector,
 struct drm_display_mode *mode)
 {
struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
+   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
+   int target_clock = mode->clock;
 
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
return MODE_NO_DBLESCAN;
@@ -212,8 +214,13 @@ intel_dvo_mode_valid(struct drm_connector *connector,
return MODE_PANEL;
if (mode->vdisplay > intel_dvo->panel_fixed_mode->vdisplay)
return MODE_PANEL;
+
+   target_clock = intel_dvo->panel_fixed_mode->clock;
}
 
+   if (target_clock > max_dotclk)
+   return MODE_CLOCK_HIGH;
+
return intel_dvo->dev.dev_ops->mode_valid(&intel_dvo->dev, mode);
 }
 
-- 
1.9.1

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

[Intel-gfx] [PATCH v5 0/4] Check pixel clock when setting mode

[Mika Kahola]

From EDID we can read and request higher pixel clock than
our HW can support. This set of patches add checks if
requested pixel clock is lower than the one supported by the HW.
The requested mode is discarded if we cannot support the requested
pixel clock. For example for Cherryview

'cvt 2560 1600 60' gives

# 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz
Modeline "2560x1600_60.00"  348.50  2560 2760 3032 3504  1600 1603 1609 1658 
-hsync +vsync

where pixel clock 348.50 MHz is higher than the supported 304 MHz.

The checks are implemented for DisplayPort, HDMI, LVDS, DVO, SDVO, DSI,
CRT, TV, and DP-MST.

V2:
- The maximum DOT clock frequency is added to debugfs i915_frequency_info.
- max dotclock cached in dev_priv structure
- moved computation of max dotclock to 'intel_display.c'

V3:
- intel_update_max_dotclk() renamed as intel_compute_max_dotclk()
- for GEN9 and above the max dotclock frequency is equal to CD clock
  frequency
- for older generations the dot clock frequency is limited to 90% of the
  CD clock frequency
- For Cherryview the dot clock is limited to 95% of CD clock frequency
- for GEN2/3 the maximum dot clock frequency is limited to 90% of the
  2X CD clock frequency as we have on option to use double wide mode
- cleanup

V4:
- renaming of max_dotclk as max_dotclk_freq in dev_priv (i915_drv.h)
  caused changes to all patches in my series even though some of them has
  been r-b'd by Ville
- for consistency the max_pixclk variable is renamed as max_dotclk throughout
  the whole series

V5:
- remaining tweaks for dotclock max frequency computation (HSW and BDW)
  and LVDS, DSI, and DVO fixes based on Ville's comments

Thanks Ville for reviewing the rest of the series!

Mika Kahola (4):
  drm/i915: Store max dotclock
  drm/i915: LVDS pixel clock check
  drm/i915: DSI pixel clock check
  drm/i915: DVO pixel clock check

 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 20 
 drivers/gpu/drm/i915/intel_dsi.c |  3 +++
 drivers/gpu/drm/i915/intel_dvo.c |  7 +++
 drivers/gpu/drm/i915/intel_lvds.c|  3 +++
 5 files changed, 34 insertions(+)

-- 
1.9.1

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

Re: [Intel-gfx] [PATCH v2] drm/i915: Put back lane_count into intel_dp and add link_rate too

[Maarten Lankhorst]

Hey,

Op 17-08-15 om 17:05 schreef ville.syrj...@linux.intel.com:
> From: Ville Syrjälä 
>
> With MST there won't be a crtc assigned to the main link encoder, so
> trying to dig up the pipe_config from there is a recipe for an oops.
>
> Instead store the parameters (lane_count and link_rate) in the encoder,
> and use those values during link training etc. Since those parameters
> are now assigned only when the link is actually enabled,
> .compute_config() won't clobber them as it did before.
>
> Hardware state readout is still bonkers though as we don't transfer the
> link parameters from pipe_config intel_dp. We should do that during
> encoder sanitation. But since we don't even do a proper job of reading
> out the main link encoder state for MST there's littel point in
> worrying about this now.
>
> Fixes a regression with MST caused by:
>  commit 90a6b7b052b1aa17fbb98b049e9c8b7f729c35a7
>  Author: Ville Syrjälä 
>  Date:   Mon Jul 6 16:39:15 2015 +0300
>
> drm/i915: Move intel_dp->lane_count into pipe_config
>
> v2: Different apporoach that should keep intel_dp_check_mst_status()
> somewhat less oopsy
>
> Cc: Maarten Lankhorst 
> Reported-by: Maarten Lankhorst 
> Signed-off-by: Ville Syrjälä 
> ---
Tested-by: Maarten Lankhorst 

Thanks, that seems to make MST work as expected again.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Avoid TP3 on CHV

[Sivakumar Thulasimani]

On 8/18/2015 12:42 PM, Jani Nikula wrote:

On Tue, 18 Aug 2015, Sivakumar Thulasimani  
wrote:

On 8/18/2015 12:14 PM, Jani Nikula wrote:

On Tue, 18 Aug 2015, Sivakumar Thulasimani  
wrote:

From: "Thulasimani,Sivakumar" 

This patch removes TP3 support on CHV since there is no support
for HBR2 on this platform.

v2: rename the function to indicate it checks source rates (Jani)

Reviewed-by: Ville Syrjälä 
Signed-off-by: Sivakumar Thulasimani 
---
   drivers/gpu/drm/i915/intel_dp.c |   24 +---
   1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 475d8cb..8bc6361 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1207,6 +1207,20 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int 
**sink_rates)
return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
   }
   
+static bool intel_dp_source_supports_hbr2(struct drm_device *dev)

+{
+   /* WaDisableHBR2:skl */
+   if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
+   return false;
+
+   if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
+   (INTEL_INFO(dev)->gen >= 9))
+   return true;
+   else
+   return false;
+}
+
+
   static int
   intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
   {
@@ -1220,12 +1234,8 @@ intel_dp_source_rates(struct drm_device *dev, const int 
**source_rates)
   
   	*source_rates = default_rates;
   
-	/* WaDisableHBR2:skl */

-   if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
-   return (DP_LINK_BW_2_7 >> 3) + 1;
-
-   if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
-   (INTEL_INFO(dev)->gen >= 9))
+   /* This depends on the fact that 5.4 is last value in the array */
+   if (intel_dp_source_supports_hbr2(dev))
return (DP_LINK_BW_5_4 >> 3) + 1;
else
return (DP_LINK_BW_2_7 >> 3) + 1;
@@ -3926,7 +3936,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
/* Training Pattern 3 support, both source and sink */
if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
-   (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)) {
+   intel_dp_source_supports_hbr2(dev)) {

hbr2 is not the same as tps3, is it? It's possible to use tps3 without
using hbr2, right?

BR,
Jani.

Yes, TP3 can be supported on panels that does not support HBR2 as well, but
the check here is for hardware capability. Intel platforms that does not
support
HBR2 cannot support TP3 as well, so we should treat them both the same.
(the only exception here seems to be SKL 
Okay, please at least add a comment to that effect.

Alternatively you add a separate intel_dp_source_supports_tps3, and
modify intel_dp_source_supports_hbr2 to be
(intel_dp_source_supports_tps3 && !WaDisableHBR2:skl). *shrug*.

BR,
Jani.


updated the patch with comments and shared again.



intel_dp->use_tps3 = true;
DRM_DEBUG_KMS("Displayport TPS3 supported\n");
} else
--
1.7.9.5


--
regards,
Sivakumar



--
regards,
Sivakumar

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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Adding Panel Filter function for DP

[Ville Syrjälä]

On Fri, Aug 14, 2015 at 07:28:44PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 14, 2015 at 05:12:57AM +, Zhang, Xiong Y wrote:
> > > On Mon, Aug 10, 2015 at 03:26:09PM +0800, Xiong Zhang wrote:
> > > > Only internal eDP, LVDS, DVI screen could set scalling mode, some
> > > > customers need to set scalling mode for external DP, HDMI, VGA screen.
> > > > Let's fulfill this.
> > > >
> > > > bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90989
> > > >
> > > > Signed-off-by: Xiong Zhang 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 63
> > > > -
> > > >  1 file changed, 44 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c index f1b9f93..2da334b 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -207,7 +207,13 @@ intel_dp_mode_valid(struct drm_connector
> > > *connector,
> > > > int target_clock = mode->clock;
> > > > int max_rate, mode_rate, max_lanes, max_link_clock;
> > > >
> > > > -   if (is_edp(intel_dp) && fixed_mode) {
> > > > +   if (mode->clock < 1)
> > > > +   return MODE_CLOCK_LOW;
> > > > +
> > > > +   if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> > > > +   return MODE_H_ILLEGAL;
> > > > +
> > > > +   if (!intel_panel_scale_none(&intel_connector->panel)) {
> > > > if (mode->hdisplay > fixed_mode->hdisplay)
> > > > return MODE_PANEL;
> > > >
> > > > @@ -226,12 +232,6 @@ intel_dp_mode_valid(struct drm_connector
> > > *connector,
> > > > if (mode_rate > max_rate)
> > > > return MODE_CLOCK_HIGH;
> > > >
> > > > -   if (mode->clock < 1)
> > > > -   return MODE_CLOCK_LOW;
> > > > -
> > > > -   if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> > > > -   return MODE_H_ILLEGAL;
> > > > -
> > > > return MODE_OK;
> > > >  }
> > > >
> > > > @@ -1378,7 +1378,7 @@ intel_dp_compute_config(struct intel_encoder
> > > *encoder,
> > > > pipe_config->has_drrs = false;
> > > > pipe_config->has_audio = intel_dp->has_audio && port != PORT_A;
> > > >
> > > > -   if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> > > > +   if (!intel_panel_scale_none(&intel_connector->panel)) {
> > > > 
> > > > intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> > > >adjusted_mode);
> > > >
> > > > @@ -4592,6 +4592,23 @@ static int intel_dp_get_modes(struct
> > > drm_connector *connector)
> > > > edid = intel_connector->detect_edid;
> > > > if (edid) {
> > > > int ret = intel_connector_update_modes(connector, edid);
> > > > +
> > > > +   if (ret && intel_connector->panel.fixed_mode == NULL) {
> > > > +   /* init fixed mode as preferred mode for DP */
> > > > +   struct drm_display_mode *fixed_mode = NULL;
> > > > +   struct drm_display_mode *scan;
> > > > +
> > > > +   list_for_each_entry(scan, 
> > > > &connector->probed_modes, head) {
> > > > +   if (scan->type & 
> > > > DRM_MODE_TYPE_PREFERRED)
> > > > +   fixed_mode = 
> > > > drm_mode_duplicate(connector->dev,
> > > > +   
> > > > scan);
> > > > +   }
> > > > +
> > > > +   if (fixed_mode)
> > > > +   
> > > > intel_panel_init(&intel_connector->panel,
> > > > +fixed_mode, NULL);
> > > > +   }
> > > 
> > > How are we supposed to get rid of a stale fixed_mode when some other
> > > display gets plugged in?
> > [Zhang, Xiong Y] Thanks so much for your good question. Yes, we should 
> > clear the
> > stale fitting_mode and fixed_mode when display is disconnect in 
> > intel_dp_hpd_pulse() function.
> > > 
> > > Also what would happen if the preferred mode can't be supported due to 
> > > some
> > > source limitation?
> > [Zhang, Xiong Y] In this case, which mode should be selected as fixed_mode ?
> 
> At the very least we should make sure it's a mode we can use.
> 
> > As you said maybe kernel isn't the right place to do such decision.
> 
> There are a lot of options how we could pick the mode. Eg. might want to
> pick the next largest mode, and if there is none try to pick the largest
> smaller mode (since pfit can't downscale by much). Also should we try to
> pick an intelaced mode if the user requested one etc. Lots of open
> questions how this policy should be handled. Would be easier to punt it
> all to userspace, which would also avoid the kernel policy doing the
> wrong thing when userspace knows what it wants.
> 
> > > 
> > > In general I'm not entirely happ

Re: [Intel-gfx] [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping

[Ville Syrjälä]

On Tue, Aug 18, 2015 at 02:28:55PM +0300, Jani Nikula wrote:
> commit 75067ddecf21271631bc018d2fb23ddd09b66aae
> Author: Antti Koskipaa 
> Date:   Fri Jul 10 14:10:55 2015 +0300
> 
> drm/i915: Per-DDI I_boost override
> 
> increased size of union child_device_config without taking into account
> the size check in parse_sdvo_device_mapping(). Switch the function over
> to using the legacy struct only.
> 
> Fixes: 75067ddecf21 ("drm/i915: Per-DDI I_boost override")
> Cc: Antti Koskipaa 
> Cc: David Weinehall 
> Signed-off-by: Jani Nikula 

Reviewed-by: Ville Syrjälä 

Also gave it a spin on my 946gz so:
Tested-by: Ville Syrjälä 

> ---
>  drivers/gpu/drm/i915/intel_bios.c | 50 
> +++
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 8e46149bafdd..1b7e1a591a37 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -401,7 +401,7 @@ parse_sdvo_device_mapping(struct drm_i915_private 
> *dev_priv,
>  {
>   struct sdvo_device_mapping *p_mapping;
>   const struct bdb_general_definitions *p_defs;
> - const union child_device_config *p_child;
> + const struct old_child_dev_config *child; /* legacy */
>   int i, child_device_num, count;
>   u16 block_size;
>  
> @@ -410,14 +410,14 @@ parse_sdvo_device_mapping(struct drm_i915_private 
> *dev_priv,
>   DRM_DEBUG_KMS("No general definition block is found, unable to 
> construct sdvo mapping.\n");
>   return;
>   }
> - /* judge whether the size of child device meets the requirements.
> -  * If the child device size obtained from general definition block
> -  * is different with sizeof(struct child_device_config), skip the
> -  * parsing of sdvo device info
> +
> + /*
> +  * Only parse SDVO mappings when the general definitions block child
> +  * device size matches that of the *legacy* child device config
> +  * struct. Thus, SDVO mapping will be skipped for newer VBT.
>*/
> - if (p_defs->child_dev_size != sizeof(*p_child)) {
> - /* different child dev size . Ignore it */
> - DRM_DEBUG_KMS("different child size is found. Invalid.\n");
> + if (p_defs->child_dev_size != sizeof(*child)) {
> + DRM_DEBUG_KMS("Unsupported child device size for SDVO 
> mapping.\n");
>   return;
>   }
>   /* get the block size of general definitions */
> @@ -427,37 +427,37 @@ parse_sdvo_device_mapping(struct drm_i915_private 
> *dev_priv,
>   p_defs->child_dev_size;
>   count = 0;
>   for (i = 0; i < child_device_num; i++) {
> - p_child = child_device_ptr(p_defs, i);
> - if (!p_child->old.device_type) {
> + child = &child_device_ptr(p_defs, i)->old;
> + if (!child->device_type) {
>   /* skip the device block if device type is invalid */
>   continue;
>   }
> - if (p_child->old.slave_addr != SLAVE_ADDR1 &&
> - p_child->old.slave_addr != SLAVE_ADDR2) {
> + if (child->slave_addr != SLAVE_ADDR1 &&
> + child->slave_addr != SLAVE_ADDR2) {
>   /*
>* If the slave address is neither 0x70 nor 0x72,
>* it is not a SDVO device. Skip it.
>*/
>   continue;
>   }
> - if (p_child->old.dvo_port != DEVICE_PORT_DVOB &&
> - p_child->old.dvo_port != DEVICE_PORT_DVOC) {
> + if (child->dvo_port != DEVICE_PORT_DVOB &&
> + child->dvo_port != DEVICE_PORT_DVOC) {
>   /* skip the incorrect SDVO port */
>   DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
>   continue;
>   }
>   DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
> - " %s port\n",
> - p_child->old.slave_addr,
> - (p_child->old.dvo_port == DEVICE_PORT_DVOB) ?
> - "SDVOB" : "SDVOC");
> - p_mapping = &(dev_priv->sdvo_mappings[p_child->old.dvo_port - 
> 1]);
> +   " %s port\n",
> +   child->slave_addr,
> +   (child->dvo_port == DEVICE_PORT_DVOB) ?
> +   "SDVOB" : "SDVOC");
> + p_mapping = &(dev_priv->sdvo_mappings[child->dvo_port - 1]);
>   if (!p_mapping->initialized) {
> - p_mapping->dvo_port = p_child->old.dvo_port;
> - p_mapping->slave_addr = p_child->old.slave_addr;
> - p_mapping->dvo_wiring = p_child->old.dvo_wiring;
> - p_mapping->ddc_pin = p

[Intel-gfx] [PATCH] drm/i915: Split alloc from init for lrc

[Nick Hoath]

Extend init/init_hw split to context init.
   - Move context initialisation in to i915_gem_init_hw
   - Move one off initialisation for render ring to
i915_gem_validate_context
   - Move default context initialisation to logical_ring_init

Rename intel_lr_context_deferred_create to
intel_lr_context_deferred_alloc, to reflect reduced functionality &
alloc/init split.

This patch is intended to split out the allocation of resources & initialisation
to allow easier reuse of code for resume/gpu reset.

v2: Removed function ptr wrapping of do_switch_context (Daniel Vetter)
Left ->init_context int intel_lr_context_deferred_alloc (Daniel Vetter)
Remove unnecessary init flag & ring type test. (Daniel Vetter)
Improve commit message (Daniel Vetter)

Issue: VIZ-4798
Signed-off-by: Nick Hoath 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_drv.h|   1 -
 drivers/gpu/drm/i915/i915_gem.c|  12 +--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   3 +-
 drivers/gpu/drm/i915/intel_lrc.c   | 147 ++---
 drivers/gpu/drm/i915/intel_lrc.h   |   4 +-
 5 files changed, 80 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f7fd519..844ccf0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -880,7 +880,6 @@ struct intel_context {
} legacy_hw_ctx;
 
/* Execlists */
-   bool rcs_initialized;
struct {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73293b4..3ccef2d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4603,14 +4603,8 @@ int i915_gem_init_rings(struct drm_device *dev)
goto cleanup_vebox_ring;
}
 
-   ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-   if (ret)
-   goto cleanup_bsd2_ring;
-
return 0;
 
-cleanup_bsd2_ring:
-   intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
@@ -4629,6 +4623,7 @@ i915_gem_init_hw(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring;
int ret, i, j;
+   struct drm_i915_gem_request *req;
 
if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
return -EIO;
@@ -4680,9 +4675,12 @@ i915_gem_init_hw(struct drm_device *dev)
goto out;
}
 
+   ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
+   if (ret)
+   goto out;
+
/* Now it is safe to go back round and do everything else: */
for_each_ring(ring, dev_priv, i) {
-   struct drm_i915_gem_request *req;
 
WARN_ON(!ring->default_context);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 923a3c4..95f1a0d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -994,6 +994,7 @@ i915_gem_validate_context(struct drm_device *dev, struct 
drm_file *file,
 {
struct intel_context *ctx = NULL;
struct i915_ctx_hang_stats *hs;
+   int ret;
 
if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
return ERR_PTR(-EINVAL);
@@ -1009,7 +1010,7 @@ i915_gem_validate_context(struct drm_device *dev, struct 
drm_file *file,
}
 
if (i915.enable_execlists && !ctx->engine[ring->id].state) {
-   int ret = intel_lr_context_deferred_create(ctx, ring);
+   ret = intel_lr_context_deferred_alloc(ctx, ring);
if (ret) {
DRM_DEBUG("Could not create LRC %u: %d\n", ctx_id, ret);
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 138964a..d0dc6b5 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1426,11 +1426,31 @@ out:
return ret;
 }
 
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
+   struct drm_i915_gem_object *default_ctx_obj)
+{
+   struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
+   /* The status page is offset 0 from the default context object
+* in LRC mode. */
+   ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj);
+   ring->status_page.page_addr =
+   kmap(sg_page(default_ctx_obj->pages->sgl));
+   ring->status_page.obj = default_ctx_obj;
+
+   I915_WRITE(RING_HWS_PGA(ring->mmio_base),
+   (u32)ring->status_page.gfx_addr);
+   POSTING_READ(RING_HWS_PGA(ring->mmio_base));
+}
+
 static int gen8_init_common_ring(struct intel_engine_cs 

Re: [Intel-gfx] [PATCH] drm/i915: Split alloc from init for lrc

[Chris Wilson]

On Tue, Aug 18, 2015 at 03:23:32PM +0100, Nick Hoath wrote:
> Extend init/init_hw split to context init.
>- Move context initialisation in to i915_gem_init_hw
>- Move one off initialisation for render ring to
> i915_gem_validate_context
>- Move default context initialisation to logical_ring_init
> 
> Rename intel_lr_context_deferred_create to
> intel_lr_context_deferred_alloc, to reflect reduced functionality &
> alloc/init split.
> 
> This patch is intended to split out the allocation of resources & 
> initialisation
> to allow easier reuse of code for resume/gpu reset.
> 
> v2: Removed function ptr wrapping of do_switch_context (Daniel Vetter)
> Left ->init_context int intel_lr_context_deferred_alloc (Daniel Vetter)
> Remove unnecessary init flag & ring type test. (Daniel Vetter)
> Improve commit message (Daniel Vetter)
> 
> Issue: VIZ-4798
> Signed-off-by: Nick Hoath 
> Cc: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/i915_drv.h|   1 -
>  drivers/gpu/drm/i915/i915_gem.c|  12 +--
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   3 +-
>  drivers/gpu/drm/i915/intel_lrc.c   | 147 
> ++---
>  drivers/gpu/drm/i915/intel_lrc.h   |   4 +-
>  5 files changed, 80 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f7fd519..844ccf0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -880,7 +880,6 @@ struct intel_context {
>   } legacy_hw_ctx;
>  
>   /* Execlists */
> - bool rcs_initialized;
>   struct {
>   struct drm_i915_gem_object *state;
>   struct intel_ringbuffer *ringbuf;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 73293b4..3ccef2d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4603,14 +4603,8 @@ int i915_gem_init_rings(struct drm_device *dev)
>   goto cleanup_vebox_ring;
>   }
>  
> - ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
> - if (ret)
> - goto cleanup_bsd2_ring;
> -
>   return 0;
>  
> -cleanup_bsd2_ring:
> - intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
>  cleanup_vebox_ring:
>   intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
>  cleanup_blt_ring:
> @@ -4629,6 +4623,7 @@ i915_gem_init_hw(struct drm_device *dev)
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct intel_engine_cs *ring;
>   int ret, i, j;
> + struct drm_i915_gem_request *req;
>  
>   if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
>   return -EIO;
> @@ -4680,9 +4675,12 @@ i915_gem_init_hw(struct drm_device *dev)
>   goto out;
>   }
>  
> + ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
> + if (ret)
> + goto out;

This is the wrong location. Just kill set_seqno, the experiment has run
its course and we now have a n igt to exercise seqno wraparound.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] lib/rendercopy_gen9: Setup Push constant pointer before sending BTP commands

[Joonas Lahtinen]

Hi,

On pe, 2015-08-14 at 10:58 +0200, Daniel Vetter wrote:
> On Thu, Aug 13, 2015 at 03:49:35PM -0700, Ben Widawsky wrote:
> > On Thu, Aug 13, 2015 at 10:33:00AM +0300, Joonas Lahtinen wrote:
> > > Hi,
> > > 
> > > On ke, 2015-08-12 at 18:35 -0700, Ben Widawsky wrote:
> > > > On Wed, Aug 12, 2015 at 03:10:18PM +0300, Joonas Lahtinen 
> > > > wrote:
> > > > > On ke, 2015-08-12 at 12:26 +0100, Arun Siluvery wrote:
> > > > > > From Gen9, by default push constant command is not 
> > > > > > committed to 
> > > > > > the 
> > > > > > shader unit
> > > > > > untill the corresponding shader's BTP_* command is parsed. 
> > > > > > This 
> > > > > > is 
> > > > > > the
> > > > > > behaviour when set shader is enabled. This patch updates 
> > > > > > the 
> > > > > > batch to 
> > > > > > follow
> > > > > > this requirement otherwise it results in gpu hang.
> > > > > > 
> > > > > > Bugzilla: 
> > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=89959
> > > > > > 
> > > > > > Set shader need to be disabled if legacy behaviour is 
> > > > > > required.
> > > > > > 
> > > > > > Cc: Ben Widawsky 
> > > > > > Cc: Joonas Lahtinen 
> > > > > > Cc: Mika Kuoppala 
> > > > > > Signed-off-by: Arun Siluvery  > > > > > >
> > > > > 
> > > > > Reviewed-by: Joonas Lahtinen  > > > > >
> > > > > 
> > > > 
> > > > Repeating what I said on the mesa thread:
> > > > Does anyone understand why this actually causes a hang on the 
> > > > IGT 
> > > > test? I
> > > > certainly don't. The docs are pretty clear that the constant 
> > > > command 
> > > > is not
> > > > committed until the BTP command, but I can't make any sense of 
> > > > how it 
> > > > related to
> > > > a GPU hang.
> > > > 
> > > 
> > > Changing the order makes the hang go away and come back for sure, 
> > > we've
> > > all been experiencing that. System validation said it is a 
> > > programming
> > > restriction, so I'm not sure how relevant it is what goes wrong 
> > > if it's
> > > not followed. And the legacy mode bits were added to support the 
> > > old
> > > behavior of having the order like it has been previously, so I do 
> > > not
> > > see why question it without visibility to the actual RTL. And 
> > > enabling
> > > the legacy bits makes the hang go away, too.
> > > 
> > > If I had the RTL sources, then it would be more relevant to take
> > > educated guesses as to why a set of hundreds of thousands of
> > > transistors doesn't work as it should :) Without that, if it gets
> > > stuck, it gets stuck.
> > > 
> > > Regards, Joonas
> > > 
> > 
> > Let me start by saying I do believe that questioning this shouldn't 
> > prevent
> > merging the patch.
> > 
> > 
> > I absolutely disagree with you and think we should question these 
> > kind of things
> > and get out of the mindset of, "well, it fixes a hang, let's move 
> > on."
> > Understanding these kind of things is critical to writing stable 
> > drivers.  If
> > the programming guide/SV team said it can lead to a hang, that's 
> > one thing, but
> > AFAICT, we do not understand why it is hanging nor does any of the 
> > documentation
> > we do have suggest it should hang. Without clarification, next time 
> > we have a
> > similar hang signature we're going to be right back here where we 
> > started.
> > 
> > It was one thing when there were a handful of us working on the 
> > stuff and we
> > didn't have time to get to the bottom of bugs like this. I'm guilty 
> > of patches
> > like this myself. I really do not see any excuse for this any more 
> > though.
> > 
> > 
> > Could you send me the reference for where SV said it was a 
> > "programming
> > restriction"? To me it all sounds very much like an implementation 
> > detail, and
> > I'd like to try to understand what I am missing.
> 
> Fully agree, we can't just ignore hangs but have to bottom out on 
> them.
> And in the past we've had piles of examples where we discovered 
> something
> and didn't chase it correctly, then a few months later massive panic 
> in
> intel. At least this must be reflected in bspec.
> 
> Joonas, can you please work together with Ben to chase this down? If 
> you
> don't get traction please escalate the shit out of this,  we really 
> must
> at least get docs to accurately reflect reality.
> 

The latest reply from SV team to Arun's inquiry is that when the order
is not followed, the previous push constant values get used (which will
be all zeroes, so doesn't really make sense) and would lead to the hang
or corruption. I'm checking them if the same applies for the BTP itself
too, where the PS pointer keeps changing (and it's the only changing
thing), and could explain it better.

Regards, Joonas

> Thanks, Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Split alloc from init for lrc

[Nick Hoath]

On 18/08/2015 15:31, Chris Wilson wrote:

On Tue, Aug 18, 2015 at 03:23:32PM +0100, Nick Hoath wrote:

Extend init/init_hw split to context init.
- Move context initialisation in to i915_gem_init_hw
- Move one off initialisation for render ring to
 i915_gem_validate_context
- Move default context initialisation to logical_ring_init

Rename intel_lr_context_deferred_create to
intel_lr_context_deferred_alloc, to reflect reduced functionality &
alloc/init split.

This patch is intended to split out the allocation of resources & initialisation
to allow easier reuse of code for resume/gpu reset.

v2: Removed function ptr wrapping of do_switch_context (Daniel Vetter)
 Left ->init_context int intel_lr_context_deferred_alloc (Daniel Vetter)
 Remove unnecessary init flag & ring type test. (Daniel Vetter)
 Improve commit message (Daniel Vetter)

Issue: VIZ-4798
Signed-off-by: Nick Hoath 
Cc: Daniel Vetter 
---
  drivers/gpu/drm/i915/i915_drv.h|   1 -
  drivers/gpu/drm/i915/i915_gem.c|  12 +--
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   3 +-
  drivers/gpu/drm/i915/intel_lrc.c   | 147 ++---
  drivers/gpu/drm/i915/intel_lrc.h   |   4 +-
  5 files changed, 80 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f7fd519..844ccf0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -880,7 +880,6 @@ struct intel_context {
} legacy_hw_ctx;

/* Execlists */
-   bool rcs_initialized;
struct {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73293b4..3ccef2d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4603,14 +4603,8 @@ int i915_gem_init_rings(struct drm_device *dev)
goto cleanup_vebox_ring;
}

-   ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
-   if (ret)
-   goto cleanup_bsd2_ring;
-
return 0;

-cleanup_bsd2_ring:
-   intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
  cleanup_vebox_ring:
intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
  cleanup_blt_ring:
@@ -4629,6 +4623,7 @@ i915_gem_init_hw(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring;
int ret, i, j;
+   struct drm_i915_gem_request *req;

if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
return -EIO;
@@ -4680,9 +4675,12 @@ i915_gem_init_hw(struct drm_device *dev)
goto out;
}

+   ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
+   if (ret)
+   goto out;


This is the wrong location. Just kill set_seqno, the experiment has run
its course and we now have a n igt to exercise seqno wraparound.
It has to be here as the seqno has to be initialised before it is used 
to create requests for the initialisation.
According to the commit history, the seqno has to be initialised to 
non-zero for proper functioning. Is this no longer true?

Maybe it should just be set to 1 instead of ~0-0x1000



-Chris



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

[Intel-gfx] [PATCH] tests/drv_hangman: Adjust to 64bit bb offsets

[Mika Kuoppala]

commit e1f123257a1f7d3af36a31a0fb2d4c6f40039fed
Author: Michel Thierry 
Date:   Wed Jul 29 17:23:56 2015 +0100

drm/i915: Expand error state's address width to 64b

changed the batch buffer address to be 64b. Fix the parsing
of gtt offset accordingly.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91638
Cc: Akash Goel 
Cc: Michel Thierry 
Signed-off-by: Mika Kuoppala 
---
 tests/drv_hangman.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
index d93bef3..070c7cf 100644
--- a/tests/drv_hangman.c
+++ b/tests/drv_hangman.c
@@ -252,7 +252,8 @@ static void check_error_state(const int gen,
while (getline(&line, &line_size, file) > 0) {
char *dashes = NULL;
int bb_matched = 0;
-   uint32_t gtt_offset;
+   uint32_t gtt_offset_upper, gtt_offset_lower;
+   uint64_t gtt_offset;
int req_matched = 0;
int requests;
uint32_t tail;
@@ -267,9 +268,11 @@ static void check_error_state(const int gen,
strncpy(ring_name, line, dashes - line);
ring_name[dashes - line - 1] = '\0';
 
-   bb_matched = sscanf(dashes, "--- gtt_offset = 0x%08x\n",
-   >t_offset);
-   if (bb_matched == 1) {
+   bb_matched = sscanf(dashes, "--- gtt_offset = 0x%08x %08x\n",
+   >t_offset_upper, >t_offset_lower);
+   gtt_offset = ((uint64_t)gtt_offset_upper << 32) | 
gtt_offset_lower;
+
+   if (bb_matched == 2) {
char expected_line[32];
 
igt_assert(strstr(ring_name, expected_ring_name));
@@ -305,7 +308,7 @@ static void check_error_state(const int gen,
}
 
ringbuf_matched = sscanf(dashes, "--- ringbuffer = 0x%08x\n",
->t_offset);
+>t_offset_lower);
if (ringbuf_matched == 1) {
unsigned int offset, command, expected_addr = 0;
 
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH] drm/i915: Split alloc from init for lrc

[Chris Wilson]

On Tue, Aug 18, 2015 at 03:55:07PM +0100, Nick Hoath wrote:
> >This is the wrong location. Just kill set_seqno, the experiment has run
> >its course and we now have a n igt to exercise seqno wraparound.
> It has to be here as the seqno has to be initialised before it is
> used to create requests for the initialisation.

It is the wrong location as init_hw() is called as part of resume and
reset, both times where we don't want to actually touch seqno.

> According to the commit history, the seqno has to be initialised to
> non-zero for proper functioning. Is this no longer true?
> Maybe it should just be set to 1 instead of ~0-0x1000

Nope. It was set to non-zero purely to test wraparound, or rather the
call to set_seqno was added purely to test wraparound. Nowadays, we can
just set it to 1 and use explicit testing to catch bugs.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] tests/drv_hangman: Adjust to 64bit bb offsets

[Michel Thierry]

On 8/18/2015 10:56 PM, Mika Kuoppala wrote:

commit e1f123257a1f7d3af36a31a0fb2d4c6f40039fed
Author: Michel Thierry 
Date:   Wed Jul 29 17:23:56 2015 +0100

 drm/i915: Expand error state's address width to 64b

changed the batch buffer address to be 64b. Fix the parsing
of gtt offset accordingly.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91638
Cc: Akash Goel 
Cc: Michel Thierry 
Signed-off-by: Mika Kuoppala 
---
  tests/drv_hangman.c | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
index d93bef3..070c7cf 100644
--- a/tests/drv_hangman.c
+++ b/tests/drv_hangman.c
@@ -252,7 +252,8 @@ static void check_error_state(const int gen,
while (getline(&line, &line_size, file) > 0) {
char *dashes = NULL;
int bb_matched = 0;
-   uint32_t gtt_offset;
+   uint32_t gtt_offset_upper, gtt_offset_lower;
+   uint64_t gtt_offset;
int req_matched = 0;
int requests;
uint32_t tail;
@@ -267,9 +268,11 @@ static void check_error_state(const int gen,
strncpy(ring_name, line, dashes - line);
ring_name[dashes - line - 1] = '\0';

-   bb_matched = sscanf(dashes, "--- gtt_offset = 0x%08x\n",
-   >t_offset);
-   if (bb_matched == 1) {
+   bb_matched = sscanf(dashes, "--- gtt_offset = 0x%08x %08x\n",
+   >t_offset_upper, >t_offset_lower);
+   gtt_offset = ((uint64_t)gtt_offset_upper << 32) | 
gtt_offset_lower;
+
+   if (bb_matched == 2) {
char expected_line[32];

igt_assert(strstr(ring_name, expected_ring_name));
@@ -305,7 +308,7 @@ static void check_error_state(const int gen,
}

ringbuf_matched = sscanf(dashes, "--- ringbuffer = 0x%08x\n",
->t_offset);
+>t_offset_lower);
if (ringbuf_matched == 1) {
unsigned int offset, command, expected_addr = 0;




Right, the error state now looks like:
 --- gtt_offset = 0x 

And
 --- ringbuffer = 0x
will have the lower 32-bits.

Reviewed-by: Michel Thierry 


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

Re: [Intel-gfx] [PATCH for v4.2] Revert "drm/i915: Allow parsing of variable size child device entries from VBT"

[Daniel Vetter]

On Tue, Aug 18, 2015 at 2:33 AM, Jani Nikula  wrote:
> This reverts
>
> commit 047fe6e6db9161e69271f56daaafdaf2add023b1
> Author: David Weinehall 
> Date:   Tue Aug 4 16:55:52 2015 +0300
>
> drm/i915: Allow parsing of variable size child device entries from VBT
>
> That commit is not valid for v4.2, however it will be valid for v4.3. It
> was simply queued too early.

Nope, this patch from David also incidentally fixes a regression from
90e4f1592bb6e82f6690f0e05a8aadc which is why I merged it to. If you
want to revert this, you need to revert more (and with that also break
BSW).

Isn't there a more minimal fix we can do for 4.2?
-Daniel

>
> The referenced regressing commit is just fine until the size of struct
> common_child_dev_config changes, and that won't happen until
> v4.3. Indeed, the expected size checks here rely on the increased size
> of the struct, breaking new platforms.
>
> Fixes: 047fe6e6db91 ("drm/i915: Allow parsing of variable size child device 
> entries from VBT")
> Cc: Daniel Vetter 
> Cc: David Weinehall 
> Cc: Ville Syrjälä 
> Signed-off-by: Jani Nikula 
>
> ---
>
> There was another patch from David increasing the size of the struct
> [1], but that then regresses sdvo mapping parsing. It's the simplest to
> just revert and fix this up properly for v4.3.
>
> [1] http://mid.gmane.org/20150813131415.GO6150@boom
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 27 ---
>  1 file changed, 4 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 3dcd59e694db..198fc3c3291b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1075,34 +1075,15 @@ parse_device_mapping(struct drm_i915_private 
> *dev_priv,
> const union child_device_config *p_child;
> union child_device_config *child_dev_ptr;
> int i, child_device_num, count;
> -   u8 expected_size;
> -   u16 block_size;
> +   u16 block_size;
>
> p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
> if (!p_defs) {
> DRM_DEBUG_KMS("No general definition block is found, no 
> devices defined.\n");
> return;
> }
> -   if (bdb->version < 195) {
> -   expected_size = 33;
> -   } else if (bdb->version == 195) {
> -   expected_size = 37;
> -   } else if (bdb->version <= 197) {
> -   expected_size = 38;
> -   } else {
> -   expected_size = 38;
> -   DRM_DEBUG_DRIVER("Expected child_device_config size for BDB 
> version %u not known; assuming %u\n",
> -expected_size, bdb->version);
> -   }
> -
> -   if (expected_size > sizeof(*p_child)) {
> -   DRM_ERROR("child_device_config cannot fit in p_child\n");
> -   return;
> -   }
> -
> -   if (p_defs->child_dev_size != expected_size) {
> -   DRM_ERROR("Size mismatch; child_device_config size=%u 
> (expected %u); bdb->version: %u\n",
> - p_defs->child_dev_size, expected_size, 
> bdb->version);
> +   if (p_defs->child_dev_size < sizeof(*p_child)) {
> +   DRM_ERROR("General definiton block child device size is too 
> small.\n");
> return;
> }
> /* get the block size of general definitions */
> @@ -1149,7 +1130,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>
> child_dev_ptr = dev_priv->vbt.child_dev + count;
> count++;
> -   memcpy(child_dev_ptr, p_child, p_defs->child_dev_size);
> +   memcpy(child_dev_ptr, p_child, sizeof(*p_child));
> }
> return;
>  }
> --
> 2.1.4
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH for v4.2] Revert "drm/i915: Allow parsing of variable size child device entries from VBT"

[Ville Syrjälä]

On Tue, Aug 18, 2015 at 09:58:57AM -0700, Daniel Vetter wrote:
> On Tue, Aug 18, 2015 at 2:33 AM, Jani Nikula  wrote:
> > This reverts
> >
> > commit 047fe6e6db9161e69271f56daaafdaf2add023b1
> > Author: David Weinehall 
> > Date:   Tue Aug 4 16:55:52 2015 +0300
> >
> > drm/i915: Allow parsing of variable size child device entries from VBT
> >
> > That commit is not valid for v4.2, however it will be valid for v4.3. It
> > was simply queued too early.
> 
> Nope, this patch from David also incidentally fixes a regression from
> 90e4f1592bb6e82f6690f0e05a8aadc which is why I merged it to.

What regression?

> If you
> want to revert this, you need to revert more (and with that also break
> BSW).
> 
> Isn't there a more minimal fix we can do for 4.2?
> -Daniel
> 
> >
> > The referenced regressing commit is just fine until the size of struct
> > common_child_dev_config changes, and that won't happen until
> > v4.3. Indeed, the expected size checks here rely on the increased size
> > of the struct, breaking new platforms.
> >
> > Fixes: 047fe6e6db91 ("drm/i915: Allow parsing of variable size child device 
> > entries from VBT")
> > Cc: Daniel Vetter 
> > Cc: David Weinehall 
> > Cc: Ville Syrjälä 
> > Signed-off-by: Jani Nikula 
> >
> > ---
> >
> > There was another patch from David increasing the size of the struct
> > [1], but that then regresses sdvo mapping parsing. It's the simplest to
> > just revert and fix this up properly for v4.3.
> >
> > [1] http://mid.gmane.org/20150813131415.GO6150@boom
> > ---
> >  drivers/gpu/drm/i915/intel_bios.c | 27 ---
> >  1 file changed, 4 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> > b/drivers/gpu/drm/i915/intel_bios.c
> > index 3dcd59e694db..198fc3c3291b 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -1075,34 +1075,15 @@ parse_device_mapping(struct drm_i915_private 
> > *dev_priv,
> > const union child_device_config *p_child;
> > union child_device_config *child_dev_ptr;
> > int i, child_device_num, count;
> > -   u8 expected_size;
> > -   u16 block_size;
> > +   u16 block_size;
> >
> > p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
> > if (!p_defs) {
> > DRM_DEBUG_KMS("No general definition block is found, no 
> > devices defined.\n");
> > return;
> > }
> > -   if (bdb->version < 195) {
> > -   expected_size = 33;
> > -   } else if (bdb->version == 195) {
> > -   expected_size = 37;
> > -   } else if (bdb->version <= 197) {
> > -   expected_size = 38;
> > -   } else {
> > -   expected_size = 38;
> > -   DRM_DEBUG_DRIVER("Expected child_device_config size for BDB 
> > version %u not known; assuming %u\n",
> > -expected_size, bdb->version);
> > -   }
> > -
> > -   if (expected_size > sizeof(*p_child)) {
> > -   DRM_ERROR("child_device_config cannot fit in p_child\n");
> > -   return;
> > -   }
> > -
> > -   if (p_defs->child_dev_size != expected_size) {
> > -   DRM_ERROR("Size mismatch; child_device_config size=%u 
> > (expected %u); bdb->version: %u\n",
> > - p_defs->child_dev_size, expected_size, 
> > bdb->version);
> > +   if (p_defs->child_dev_size < sizeof(*p_child)) {
> > +   DRM_ERROR("General definiton block child device size is too 
> > small.\n");
> > return;
> > }
> > /* get the block size of general definitions */
> > @@ -1149,7 +1130,7 @@ parse_device_mapping(struct drm_i915_private 
> > *dev_priv,
> >
> > child_dev_ptr = dev_priv->vbt.child_dev + count;
> > count++;
> > -   memcpy(child_dev_ptr, p_child, p_defs->child_dev_size);
> > +   memcpy(child_dev_ptr, p_child, sizeof(*p_child));
> > }
> > return;
> >  }
> > --
> > 2.1.4
> >
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH for v4.2] Revert "drm/i915: Allow parsing of variable size child device entries from VBT"

[Daniel Vetter]

On Tue, Aug 18, 2015 at 10:00 AM, Ville Syrjälä
 wrote:
> On Tue, Aug 18, 2015 at 09:58:57AM -0700, Daniel Vetter wrote:
>> On Tue, Aug 18, 2015 at 2:33 AM, Jani Nikula  wrote:
>> > This reverts
>> >
>> > commit 047fe6e6db9161e69271f56daaafdaf2add023b1
>> > Author: David Weinehall 
>> > Date:   Tue Aug 4 16:55:52 2015 +0300
>> >
>> > drm/i915: Allow parsing of variable size child device entries from VBT
>> >
>> > That commit is not valid for v4.2, however it will be valid for v4.3. It
>> > was simply queued too early.
>>
>> Nope, this patch from David also incidentally fixes a regression from
>> 90e4f1592bb6e82f6690f0e05a8aadc which is why I merged it to.
>
> What regression?

Quoting the commit message: "...  since we're hitting a DRM_ERROR on
older platforms with this." Not every platform has the BSW vbt layout
;-) Oh and why do I even bother to write this stuff when no one reads
it?
-Daniel

>> If you
>> want to revert this, you need to revert more (and with that also break
>> BSW).
>>
>> Isn't there a more minimal fix we can do for 4.2?
>> -Daniel
>>
>> >
>> > The referenced regressing commit is just fine until the size of struct
>> > common_child_dev_config changes, and that won't happen until
>> > v4.3. Indeed, the expected size checks here rely on the increased size
>> > of the struct, breaking new platforms.
>> >
>> > Fixes: 047fe6e6db91 ("drm/i915: Allow parsing of variable size child 
>> > device entries from VBT")
>> > Cc: Daniel Vetter 
>> > Cc: David Weinehall 
>> > Cc: Ville Syrjälä 
>> > Signed-off-by: Jani Nikula 
>> >
>> > ---
>> >
>> > There was another patch from David increasing the size of the struct
>> > [1], but that then regresses sdvo mapping parsing. It's the simplest to
>> > just revert and fix this up properly for v4.3.
>> >
>> > [1] http://mid.gmane.org/20150813131415.GO6150@boom
>> > ---
>> >  drivers/gpu/drm/i915/intel_bios.c | 27 ---
>> >  1 file changed, 4 insertions(+), 23 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>> > b/drivers/gpu/drm/i915/intel_bios.c
>> > index 3dcd59e694db..198fc3c3291b 100644
>> > --- a/drivers/gpu/drm/i915/intel_bios.c
>> > +++ b/drivers/gpu/drm/i915/intel_bios.c
>> > @@ -1075,34 +1075,15 @@ parse_device_mapping(struct drm_i915_private 
>> > *dev_priv,
>> > const union child_device_config *p_child;
>> > union child_device_config *child_dev_ptr;
>> > int i, child_device_num, count;
>> > -   u8 expected_size;
>> > -   u16 block_size;
>> > +   u16 block_size;
>> >
>> > p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
>> > if (!p_defs) {
>> > DRM_DEBUG_KMS("No general definition block is found, no 
>> > devices defined.\n");
>> > return;
>> > }
>> > -   if (bdb->version < 195) {
>> > -   expected_size = 33;
>> > -   } else if (bdb->version == 195) {
>> > -   expected_size = 37;
>> > -   } else if (bdb->version <= 197) {
>> > -   expected_size = 38;
>> > -   } else {
>> > -   expected_size = 38;
>> > -   DRM_DEBUG_DRIVER("Expected child_device_config size for 
>> > BDB version %u not known; assuming %u\n",
>> > -expected_size, bdb->version);
>> > -   }
>> > -
>> > -   if (expected_size > sizeof(*p_child)) {
>> > -   DRM_ERROR("child_device_config cannot fit in p_child\n");
>> > -   return;
>> > -   }
>> > -
>> > -   if (p_defs->child_dev_size != expected_size) {
>> > -   DRM_ERROR("Size mismatch; child_device_config size=%u 
>> > (expected %u); bdb->version: %u\n",
>> > - p_defs->child_dev_size, expected_size, 
>> > bdb->version);
>> > +   if (p_defs->child_dev_size < sizeof(*p_child)) {
>> > +   DRM_ERROR("General definiton block child device size is 
>> > too small.\n");
>> > return;
>> > }
>> > /* get the block size of general definitions */
>> > @@ -1149,7 +1130,7 @@ parse_device_mapping(struct drm_i915_private 
>> > *dev_priv,
>> >
>> > child_dev_ptr = dev_priv->vbt.child_dev + count;
>> > count++;
>> > -   memcpy(child_dev_ptr, p_child, p_defs->child_dev_size);
>> > +   memcpy(child_dev_ptr, p_child, sizeof(*p_child));
>> > }
>> > return;
>> >  }
>> > --
>> > 2.1.4
>> >
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
> --
> Ville Syrjälä
> Intel OTC



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/guc: Support GuC version 4.3

[yu . dai]

From: Alex Dai 

The firmware layout changes that now it only has css header +
uCode + RSA signature. Plus, other trivial changes to support
GuC V4.3.

Signed-off-by: Alex Dai 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 11 ---
 drivers/gpu/drm/i915/intel_guc_loader.c | 17 ++---
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 950c7e7..e1f47ba 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -41,7 +41,7 @@
 #define GUC_CTX_PRIORITY_NORMAL3
 
 #define GUC_MAX_GPU_CONTEXTS   1024
-#defineGUC_INVALID_CTX_ID  (GUC_MAX_GPU_CONTEXTS + 1)
+#defineGUC_INVALID_CTX_ID  GUC_MAX_GPU_CONTEXTS
 
 /* Work queue item header definitions */
 #define WQ_STATUS_ACTIVE   1
@@ -75,6 +75,7 @@
 #define GUC_CTX_DESC_ATTR_RESET(1 << 4)
 #define GUC_CTX_DESC_ATTR_WQLOCKED (1 << 5)
 #define GUC_CTX_DESC_ATTR_PCH  (1 << 6)
+#define GUC_CTX_DESC_ATTR_TERMINATED   (1 << 7)
 
 /* The guc control data is 10 DWORDs */
 #define GUC_CTL_CTXINFO0
@@ -107,6 +108,7 @@
 #define   GUC_CTL_DISABLE_SCHEDULER(1 << 4)
 #define   GUC_CTL_PREEMPTION_LOG   (1 << 5)
 #define   GUC_CTL_ENABLE_SLPC  (1 << 7)
+#define   GUC_CTL_RESET_ON_PREMPT_FAILURE  (1 << 8)
 #define GUC_CTL_DEBUG  8
 #define   GUC_LOG_VERBOSITY_SHIFT  0
 #define   GUC_LOG_VERBOSITY_LOW(0 << GUC_LOG_VERBOSITY_SHIFT)
@@ -116,8 +118,9 @@
 /* Verbosity range-check limits, without the shift */
 #define  GUC_LOG_VERBOSITY_MIN 0
 #define  GUC_LOG_VERBOSITY_MAX 3
+#define GUC_CTL_RSRVD  9
 
-#define GUC_CTL_MAX_DWORDS (GUC_CTL_DEBUG + 1)
+#define GUC_CTL_MAX_DWORDS (GUC_CTL_RSRVD + 1)
 
 struct guc_doorbell_info {
u32 db_status;
@@ -207,7 +210,9 @@ struct guc_context_desc {
 
u32 engine_presence;
 
-   u32 reserved0[1];
+   u8 engine_suspended;
+
+   u8 reserved0[3];
u64 reserved1[1];
 
u64 desc_private;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index d46195c..7521eac 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -59,7 +59,7 @@
  *
  */
 
-#define I915_SKL_GUC_UCODE "i915/skl_guc_ver3.bin"
+#define I915_SKL_GUC_UCODE "i915/skl_guc_ver4.bin"
 MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
 
 /* User-friendly representation of an enum */
@@ -226,10 +226,6 @@ static inline bool guc_ucode_response(struct 
drm_i915_private *dev_priv,
  * +---+  
  * | RSA signature |  256B
  * +---+  
- * | RSA public Key|  256B
- * +---+  
- * |   Public key modulus  |4B
- * +---+  
  *
  * Architecturally, the DMA engine is bidirectional, and can potentially even
  * transfer between GTT locations. This functionality is left out of the API
@@ -244,7 +240,6 @@ static inline bool guc_ucode_response(struct 
drm_i915_private *dev_priv,
 #define UOS_VER_MAJOR_OFFSET   0x46
 #define UOS_CSS_HEADER_SIZE0x80
 #define UOS_RSA_SIG_SIZE   0x100
-#define UOS_CSS_SIGNING_SIZE   0x204
 
 static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
 {
@@ -256,7 +251,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private 
*dev_priv)
int i, ret = 0;
 
/* uCode size, also is where RSA signature starts */
-   offset = ucode_size = guc_fw->guc_fw_size - UOS_CSS_SIGNING_SIZE;
+   offset = ucode_size = guc_fw->guc_fw_size - UOS_RSA_SIG_SIZE;
I915_WRITE(DMA_COPY_SIZE, ucode_size);
 
/* Copy RSA signature from the fw image to HW for verification */
@@ -471,8 +466,8 @@ static void guc_fw_fetch(struct drm_device *dev, struct 
intel_guc_fw *guc_fw)
struct drm_i915_gem_object *obj;
const struct firmware *fw;
const u8 *css_header;
-   const size_t minsize = UOS_CSS_HEADER_SIZE + UOS_CSS_SIGNING_SIZE;
-   const size_t maxsize = GUC_WOPCM_SIZE_VALUE + UOS_CSS_SIGNING_SIZE
+   const size_t minsize = UOS_CSS_HEADER_SIZE + UOS_RSA_SIG_SIZE;
+   const size_t maxsize = GUC_WOPCM_SIZE_VALUE + UOS_RSA_SIG_SIZE
- 0x8000; /* 32k reserved (8K stack + 24k context) */
int err;
 
@@ -572,8 +567,8 @@ void intel_guc_ucode_init(struct drm_device *dev)
fw_path = NULL;
} else if (IS_SKYLAKE(dev)) {
fw_path = I915_SKL_GUC_UCODE;
-   guc_fw->guc_fw_major_wanted = 3;
-   guc_fw->guc_fw_minor_wanted = 0;
+   guc_fw->guc_fw_major_wanted = 4;
+   guc_fw->guc_fw_minor_wanted = 3;
} else

[Intel-gfx] [PATCH] drm/i915: Notify GuC rc6 state

[yu . dai]

From: Alex Dai 

If rc6 is enabled, notify GuC so it can do proper forcewake before
command submission.

Signed-off-by: Alex Dai 
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index ec70393..792d0b9 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -151,6 +151,18 @@ static int host2guc_release_doorbell(struct intel_guc *guc,
return host2guc_action(guc, data, 2);
 }
 
+static int host2guc_sample_forcewake(struct intel_guc *guc,
+struct i915_guc_client *client)
+{
+   struct drm_i915_private *dev_priv = guc_to_i915(guc);
+   u32 data[2];
+
+   data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE;
+   data[1] = (intel_enable_rc6(dev_priv->dev)) ? 1 : 0;
+
+   return host2guc_action(guc, data, 2);
+}
+
 /*
  * Initialise, update, or clear doorbell data shared with the GuC
  *
@@ -874,6 +886,9 @@ int i915_guc_submission_enable(struct drm_device *dev)
}
 
guc->execbuf_client = client;
+
+   host2guc_sample_forcewake(guc, client);
+
return 0;
 }
 
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 08/16] drm/i915: avoid the last 8mb of stolen on BDW/SKL

[Zanoni, Paulo R]

Em Sáb, 2015-08-15 às 09:29 +0100, Chris Wilson escreveu:
> On Fri, Aug 14, 2015 at 06:34:13PM -0300, Paulo Zanoni wrote:
> > The FBC hardware for these platforms doesn't have access to the
> > bios_reserved range, so it always assumes the maximum (8mb) is 
> > used.
> > So avoid this range while allocating.
> > 
> > This solves a bunch of FIFO underruns that happen if you end up
> > putting the CFB in that memory range. On my machine, with 32mb of
> > stolen, I need a 2560x1440 mode for that.
> 
> If the restriction applies only to FBC, apply it to only fbc.

That's what the patch is doing. The part where we set the unusual
start/end is at intel_fbc.c:find_compression_threshold(). Everything
else should be behaving just as before.

Maybe you're being confused by the addition of the stolen_usable_size
variable.

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

Re: [Intel-gfx] [PATCH 15/16] Revert "drm/i915: Allocate fbcon from stolen memory"

[Zanoni, Paulo R]

Em Sáb, 2015-08-15 às 09:24 +0100, Chris Wilson escreveu:
> On Fri, Aug 14, 2015 at 06:34:20PM -0300, Paulo Zanoni wrote:
> > This reverts commit 0ffb0ff283cca16f72caf29c44496d83b0c291fb.
> > 
> > Technology has evolved and now we have eDP panels with 3200x1800
> > resolution. In the meantime, the BIOS guys didn't change the 
> > default
> > 32mb for stolen memory. And we can't assume our users will be able 
> > to
> > increase the default stolen memory size to more than 32mb - I'm not
> > even sure all BIOSes allow that.
> > 
> > So just the fbcon buffer alone eats 22mb of my stolen memroy, and 
> > due
> > to the BDW/SKL restriction of not using the last 8mb of stolen 
> > memory,
> > all that's left for FBC is 2mb! Since fbcon is not the coolest 
> > feature
> > ever, I think it's better to save our precious stolen resource to 
> > FBC
> > and the other guys.
> > 
> > Besides, neither the original commit message nor the review 
> > comments
> > showed any arguments in favor of actually allocating fbcon from
> > stolen.
> 
> Pardon?


pzanoni@panetone:~/git/kernel/kernel$ git show
0ffb0ff283cca16f72caf29c44496d83b0c291fb | head -n 12
commit 0ffb0ff283cca16f72caf29c44496d83b0c291fb
Author: Chris Wilson 
Date:   Thu Nov 15 11:32:27 2012 +

drm/i915: Allocate fbcon from stolen memory

Signed-off-by: Chris Wilson 
Reviewed-by: Jesse Barnes 
Acked-by: Ben Widawsky 
Signed-off-by: Daniel Vetter 

diff --git a/drivers/gpu/drm/i915/intel_fb.c
b/drivers/gpu/drm/i915/intel_fb.c
pzanoni@panetone:~/git/kernel/kernel$ 

And the only review I could find was: 

http://lists.freedesktop.org/archives/intel-gfx/2012
-October/021025.html

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

Re: [Intel-gfx] [PATCH] lib/rendercopy_gen9: Setup Push constant pointer before sending BTP commands

[Ben Widawsky]

On Tue, Aug 18, 2015 at 05:25:55PM +0300, Joonas Lahtinen wrote:
> Hi,
> 
> On pe, 2015-08-14 at 10:58 +0200, Daniel Vetter wrote:
> > On Thu, Aug 13, 2015 at 03:49:35PM -0700, Ben Widawsky wrote:
> > > On Thu, Aug 13, 2015 at 10:33:00AM +0300, Joonas Lahtinen wrote:
> > > > Hi,
> > > > 
> > > > On ke, 2015-08-12 at 18:35 -0700, Ben Widawsky wrote:
> > > > > On Wed, Aug 12, 2015 at 03:10:18PM +0300, Joonas Lahtinen 
> > > > > wrote:
> > > > > > On ke, 2015-08-12 at 12:26 +0100, Arun Siluvery wrote:
> > > > > > > From Gen9, by default push constant command is not 
> > > > > > > committed to 
> > > > > > > the 
> > > > > > > shader unit
> > > > > > > untill the corresponding shader's BTP_* command is parsed. 
> > > > > > > This 
> > > > > > > is 
> > > > > > > the
> > > > > > > behaviour when set shader is enabled. This patch updates 
> > > > > > > the 
> > > > > > > batch to 
> > > > > > > follow
> > > > > > > this requirement otherwise it results in gpu hang.
> > > > > > > 
> > > > > > > Bugzilla: 
> > > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=89959
> > > > > > > 
> > > > > > > Set shader need to be disabled if legacy behaviour is 
> > > > > > > required.
> > > > > > > 
> > > > > > > Cc: Ben Widawsky 
> > > > > > > Cc: Joonas Lahtinen 
> > > > > > > Cc: Mika Kuoppala 
> > > > > > > Signed-off-by: Arun Siluvery  > > > > > > >
> > > > > > 
> > > > > > Reviewed-by: Joonas Lahtinen  > > > > > >
> > > > > > 
> > > > > 
> > > > > Repeating what I said on the mesa thread:
> > > > > Does anyone understand why this actually causes a hang on the 
> > > > > IGT 
> > > > > test? I
> > > > > certainly don't. The docs are pretty clear that the constant 
> > > > > command 
> > > > > is not
> > > > > committed until the BTP command, but I can't make any sense of 
> > > > > how it 
> > > > > related to
> > > > > a GPU hang.
> > > > > 
> > > > 
> > > > Changing the order makes the hang go away and come back for sure, 
> > > > we've
> > > > all been experiencing that. System validation said it is a 
> > > > programming
> > > > restriction, so I'm not sure how relevant it is what goes wrong 
> > > > if it's
> > > > not followed. And the legacy mode bits were added to support the 
> > > > old
> > > > behavior of having the order like it has been previously, so I do 
> > > > not
> > > > see why question it without visibility to the actual RTL. And 
> > > > enabling
> > > > the legacy bits makes the hang go away, too.
> > > > 
> > > > If I had the RTL sources, then it would be more relevant to take
> > > > educated guesses as to why a set of hundreds of thousands of
> > > > transistors doesn't work as it should :) Without that, if it gets
> > > > stuck, it gets stuck.
> > > > 
> > > > Regards, Joonas
> > > > 
> > > 
> > > Let me start by saying I do believe that questioning this shouldn't 
> > > prevent
> > > merging the patch.
> > > 
> > > 
> > > I absolutely disagree with you and think we should question these 
> > > kind of things
> > > and get out of the mindset of, "well, it fixes a hang, let's move 
> > > on."
> > > Understanding these kind of things is critical to writing stable 
> > > drivers.  If
> > > the programming guide/SV team said it can lead to a hang, that's 
> > > one thing, but
> > > AFAICT, we do not understand why it is hanging nor does any of the 
> > > documentation
> > > we do have suggest it should hang. Without clarification, next time 
> > > we have a
> > > similar hang signature we're going to be right back here where we 
> > > started.
> > > 
> > > It was one thing when there were a handful of us working on the 
> > > stuff and we
> > > didn't have time to get to the bottom of bugs like this. I'm guilty 
> > > of patches
> > > like this myself. I really do not see any excuse for this any more 
> > > though.
> > > 
> > > 
> > > Could you send me the reference for where SV said it was a 
> > > "programming
> > > restriction"? To me it all sounds very much like an implementation 
> > > detail, and
> > > I'd like to try to understand what I am missing.
> > 
> > Fully agree, we can't just ignore hangs but have to bottom out on 
> > them.
> > And in the past we've had piles of examples where we discovered 
> > something
> > and didn't chase it correctly, then a few months later massive panic 
> > in
> > intel. At least this must be reflected in bspec.
> > 
> > Joonas, can you please work together with Ben to chase this down? If 
> > you
> > don't get traction please escalate the shit out of this,  we really 
> > must
> > at least get docs to accurately reflect reality.
> > 
> 
> The latest reply from SV team to Arun's inquiry is that when the order
> is not followed, the previous push constant values get used (which will
> be all zeroes, so doesn't really make sense) and would lead to the hang
> or corruption. I'm checking them if the same applies for the BTP itself
> too, where the PS pointer keeps changing (and it's the only changing
> thing), and could explain it better.
> 
> Regards, Joona

Re: [Intel-gfx] [PATCH 08/15] drm/i915: Implement PHY lane power gating for CHV

[Deepak]

On 07/09/2015 02:15 AM, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä 

Powergate the PHY lanes when they're not needed. For HDMI all four lanes
are needed always, but for DP we can enable only the needed lanes. To
power down the unused lanes we use some power down override bits in the
DISPLAY_PHY_CONTROL register. Without the overrides it appears that the
hardware always powers on all the lanes. When the port is disabled the
power down override is not needed and the lanes will shut off on their
own. That also means the override is critical to actually be able to
access the DPIO registers before the port is actually enabled.

Additionally the common lanes will power down when not needed. CL1
remains on as long as anything else is on, CL2 will shut down when
all the lanes in the same channel will shut down. There is one exception
for CL2 that will be dealt in a separate patch for clarity.

With potentially some lanes powered down, the DP code now has to check
the number of active lanes before accessing PCS/TX registers. All
registers in powered down blocks will reads as 0x, and soe we
would drown in warnings from vlv_dpio_read() if we allowed the code
to access all those registers.

Another important detail in the DP code is the "TX latency optimal"
setting. Normally the second TX lane acts as some kind of reset master,
with the other lanes as slaves. But when only a single lane is enabled,
that single lane obviously has to be the master.

A bit of extra care is needed to reconstruct the initial state of the
DISPLAY_PHY_CONTROL register since it can't be read safely. So instead
read the actual lane status from the DPLL/PHY_STATUS registers and
use that to determine which lanes ought to be powergated initially.

We also need to switch the PHY power modes to "deep PSR" to avoid
a hard system hang when powering down the single channel PHY.

Also sprinkle a few debug prints around so that we can monitor the
DISPLAY_PHY_STATUS changes without having to read it and risk
corrupting it.

v2: Add locking to chv_powergate_phy_lanes()
v3: Actually enable dynamic powerdown in the PHY and deal with the
 fallout

Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/i915_reg.h |   8 ++
  drivers/gpu/drm/i915/intel_dp.c | 141 +---
  drivers/gpu/drm/i915/intel_drv.h|   4 +
  drivers/gpu/drm/i915/intel_hdmi.c   |   4 +
  drivers/gpu/drm/i915/intel_runtime_pm.c | 123 ++--
  5 files changed, 221 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 17cb7e5..bcfcbb62 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1125,9 +1125,15 @@ enum skl_disp_power_wells {
  
  #define _CHV_CMN_DW19_CH0		0x814c

  #define _CHV_CMN_DW6_CH1  0x8098
+#define   DPIO_DYNPWRDOWNEN_CH1(1 << 28) /* CL2 DW6 only */
  #define   CHV_CMN_USEDCLKCHANNEL  (1 << 13)
+
  #define CHV_CMN_DW19(ch) _PIPE(ch, _CHV_CMN_DW19_CH0, _CHV_CMN_DW6_CH1)
  
+#define CHV_CMN_DW28			0x8170

+#define   DPIO_CL1POWERDOWNEN  (1 << 23)
+#define   DPIO_DYNPWRDOWNEN_CH0(1 << 22)
+
  #define CHV_CMN_DW30  0x8178
  #define   DPIO_LRC_BYPASS (1 << 3)
  
@@ -2175,10 +2181,12 @@ enum skl_disp_power_wells {

  #define DPIO_PHY_STATUS   (VLV_DISPLAY_BASE + 0x6240)
  #define   DPLL_PORTD_READY_MASK   (0xf)
  #define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
+#define   PHY_CH_POWER_DOWN_OVRD_EN(phy, ch)   (1 << (2*(phy)+(ch)+27))
  #define   PHY_LDO_DELAY_0NS   0x0
  #define   PHY_LDO_DELAY_200NS 0x1
  #define   PHY_LDO_DELAY_600NS 0x2
  #define   PHY_LDO_SEQ_DELAY(delay, phy)   ((delay) << 
(2*(phy)+23))
+#define   PHY_CH_POWER_DOWN_OVRD(mask, phy, ch)((mask) << 
(8*(phy)+4*(ch)+11))
  #define   PHY_CH_SU_PSR   0x1
  #define   PHY_CH_DEEP_PSR 0x7
  #define   PHY_CH_POWER_MODE(mode, phy, ch)((mode) << (6*(phy)+3*(ch)+2))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 40b8430..6058129 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -133,6 +133,11 @@ static void vlv_init_panel_power_sequencer(struct intel_dp 
*intel_dp);
  static void vlv_steal_power_sequencer(struct drm_device *dev,
  enum pipe pipe);
  
+static unsigned int intel_dp_unused_lane_mask(int lane_count)

+{
+   return ~((1 << lane_count) - 1) & 0xf;
+}
+
  static int
  intel_dp_max_link_bw(struct intel_dp  *intel_dp)
  {
@@ -2395,17 +2400,21 @@ static void chv_post_disable_dp(struct intel_encoder 
*encoder)
val |= CHV_PCS_REQ_SOFTRESET_EN;
vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW1(ch), val);
  
-	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW

Re: [Intel-gfx] [PATCH 09/15] drm/i915: Trick CL2 into life on CHV when using pipe B with port B

[Deepak]

On 07/09/2015 02:15 AM, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä 

Normmally the common lane in a PHY channel gets powered up when some
of the data lanes get powered up. But when we're driving port B with
pipe B we don't want to enabled any of the data lanes, and just want
the DPLL in the common lane to be active.

To make that happens we have to temporarily enable some data lanes
after which we can access the DPLL registers in the common lane. Once
the pipe is up and running we can drop the power override on the data
lanes allowing them to shut down. From this point forward the common
lane will in fact stay powered on until the data lanes in the other
channel get powered down.
Patch looks fine. It does what it says. One Q, why only for port B? Port 
C is also in same common lane right?

Reviewed-by: Deepak S 

Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/intel_dp.c | 23 +++
  drivers/gpu/drm/i915/intel_drv.h|  3 +++
  drivers/gpu/drm/i915/intel_hdmi.c   | 23 +++
  drivers/gpu/drm/i915/intel_runtime_pm.c | 29 +
  4 files changed, 78 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6058129..8d088f3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2865,6 +2865,12 @@ static void chv_pre_enable_dp(struct intel_encoder 
*encoder)
mutex_unlock(&dev_priv->sb_lock);
  
  	intel_enable_dp(encoder);

+
+   /* Second common lane will stay alive on its own now */
+   if (dport->release_cl2_override) {
+   chv_phy_powergate_ch(dev_priv, DPIO_PHY0, DPIO_CH1, false);
+   dport->release_cl2_override = false;
+   }
  }
  
  static void chv_dp_pre_pll_enable(struct intel_encoder *encoder)

@@ -2882,6 +2888,14 @@ static void chv_dp_pre_pll_enable(struct intel_encoder 
*encoder)
  
  	intel_dp_prepare(encoder);
  
+	/*

+* Must trick the second common lane into life.
+* Otherwise we can't even access the PLL.
+*/
+   if (ch == DPIO_CH0 && pipe == PIPE_B)
+   dport->release_cl2_override =
+   !chv_phy_powergate_ch(dev_priv, DPIO_PHY0, DPIO_CH1, 
true);
+
chv_phy_powergate_lanes(encoder, true, lane_mask);
  
  	mutex_lock(&dev_priv->sb_lock);

@@ -2960,6 +2974,15 @@ static void chv_dp_post_pll_disable(struct intel_encoder 
*encoder)
  
  	mutex_unlock(&dev_priv->sb_lock);
  
+	/*

+* Leave the power down bit cleared for at least one
+* lane so that chv_powergate_phy_ch() will power
+* on something when the channel is otherwise unused.
+* When the port is off and the override is removed
+* the lanes power down anyway, so otherwise it doesn't
+* really matter what the state of power down bits is
+* after this.
+*/
chv_phy_powergate_lanes(encoder, false, 0x0);
  }
  
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h

index f8a16dc..6133a98 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -782,6 +782,7 @@ struct intel_digital_port {
struct intel_dp dp;
struct intel_hdmi hdmi;
enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
+   bool release_cl2_override;
  };
  
  struct intel_dp_mst_encoder {

@@ -1372,6 +1373,8 @@ void intel_display_set_init_power(struct drm_i915_private 
*dev, bool enable);
  
  void chv_phy_powergate_lanes(struct intel_encoder *encoder,

 bool override, unsigned int mask);
+bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
+ enum dpio_channel ch, bool override);
  
  
  /* intel_pm.c */

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index b3f6c9f..4b604ee 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1625,6 +1625,14 @@ static void chv_hdmi_pre_pll_enable(struct intel_encoder 
*encoder)
  
  	intel_hdmi_prepare(encoder);
  
+	/*

+* Must trick the second common lane into life.
+* Otherwise we can't even access the PLL.
+*/
+   if (ch == DPIO_CH0 && pipe == PIPE_B)
+   dport->release_cl2_override =
+   !chv_phy_powergate_ch(dev_priv, DPIO_PHY0, DPIO_CH1, 
true);
+
chv_phy_powergate_lanes(encoder, true, 0x0);
  
  	mutex_lock(&dev_priv->sb_lock);

@@ -1701,6 +1709,15 @@ static void chv_hdmi_post_pll_disable(struct 
intel_encoder *encoder)
  
  	mutex_unlock(&dev_priv->sb_lock);
  
+	/*

+* Leave the power down bit cleared for at least one
+* lane so that chv_powergate_phy_ch() will power
+* on something when the channel is otherwise unused.
+* When the port is off and the override is removed
+* the lanes power down anyway, so otherwise it doesn't
+  

Re: [Intel-gfx] [PATCH 10/15] drm/i915: Force common lane on for the PPS kick on CHV

[Deepak]

On 07/09/2015 02:15 AM, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä 

With DPIO powergating active the DPLL can't be accessed unless
something else is keeping the common lane in the channel on.
That means the PPS kick procedure could fail to enable the PLL.

Power up some data lanes to force the common lane to power up
so that the PLL can be enabled temporarily.

Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/intel_dp.c | 16 +---
  1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8d088f3..817df87 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -341,7 +341,9 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
enum pipe pipe = intel_dp->pps_pipe;
-   bool pll_enabled;
+   bool pll_enabled, release_cl_override;
+   enum dpio_phy phy = DPIO_PHY(pipe);
+   enum dpio_channel ch = vlv_pipe_to_channel(pipe);
uint32_t DP;
  
  	if (WARN(I915_READ(intel_dp->output_reg) & DP_PORT_EN,

@@ -371,9 +373,13 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
 * The DPLL for the pipe must be enabled for this to work.
 * So enable temporarily it if it's not already enabled.
 */
-   if (!pll_enabled)
+   if (!pll_enabled) {
+   release_cl_override = IS_CHERRYVIEW(dev) &&
+   !chv_phy_powergate_ch(dev_priv, phy, ch, true);
+
vlv_force_pll_on(dev, pipe, IS_CHERRYVIEW(dev) ?
 &chv_dpll[0].dpll : &vlv_dpll[0].dpll);
+   }
  
  	/*

 * Similar magic as in intel_dp_enable_port().
@@ -390,8 +396,12 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
I915_WRITE(intel_dp->output_reg, DP & ~DP_PORT_EN);
POSTING_READ(intel_dp->output_reg);
  
-	if (!pll_enabled)

+   if (!pll_enabled) {
vlv_force_pll_off(dev, pipe);
+
+   if (release_cl_override)
+   chv_phy_powergate_ch(dev_priv, phy, ch, false);
+   }
  }
  
  static enum pipe

Change looks fine
Reviewed-by: Deepak S 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH for v4.2] Revert "drm/i915: Allow parsing of variable size child device entries from VBT"

[Jani Nikula]

On Wed, 19 Aug 2015, Daniel Vetter  wrote:
> On Tue, Aug 18, 2015 at 10:00 AM, Ville Syrjälä
>  wrote:
>> On Tue, Aug 18, 2015 at 09:58:57AM -0700, Daniel Vetter wrote:
>>> On Tue, Aug 18, 2015 at 2:33 AM, Jani Nikula  wrote:
>>> > This reverts
>>> >
>>> > commit 047fe6e6db9161e69271f56daaafdaf2add023b1
>>> > Author: David Weinehall 
>>> > Date:   Tue Aug 4 16:55:52 2015 +0300
>>> >
>>> > drm/i915: Allow parsing of variable size child device entries from VBT
>>> >
>>> > That commit is not valid for v4.2, however it will be valid for v4.3. It
>>> > was simply queued too early.
>>>
>>> Nope, this patch from David also incidentally fixes a regression from
>>> 90e4f1592bb6e82f6690f0e05a8aadc which is why I merged it to.
>>
>> What regression?
>
> Quoting the commit message: "...  since we're hitting a DRM_ERROR on
> older platforms with this." Not every platform has the BSW vbt layout
> ;-) Oh and why do I even bother to write this stuff when no one reads
> it?

I read it, and I think it's wrong. I even explained why in my commit
message (yes, please read it). I don't think there was a DRM_ERROR on
older platform with Ville's patch, on v4.2 where the revert is targeted,
and even if there were, David's patch would *not* fix it. Indeed there
was no discussion of a regression on the mailing list.

If there was any regression, it was introduced by

commit 75067ddecf21271631bc018d2fb23ddd09b66aae
Author: Antti Koskipaa 
Date:   Fri Jul 10 14:10:55 2015 +0300

drm/i915: Per-DDI I_boost override

when the size of union child_device_config changed. But that's headed
for v4.3. And we're talking about v4.2, which is getting pretty
urgent. We'll have a bit more time to sort out the mess that will land
in v4.3 (and I've already sent one patch sorting out SDVO breakage).

BR,
Jani.



> -Daniel
>
>>> If you
>>> want to revert this, you need to revert more (and with that also break
>>> BSW).
>>>
>>> Isn't there a more minimal fix we can do for 4.2?
>>> -Daniel
>>>
>>> >
>>> > The referenced regressing commit is just fine until the size of struct
>>> > common_child_dev_config changes, and that won't happen until
>>> > v4.3. Indeed, the expected size checks here rely on the increased size
>>> > of the struct, breaking new platforms.
>>> >
>>> > Fixes: 047fe6e6db91 ("drm/i915: Allow parsing of variable size child 
>>> > device entries from VBT")
>>> > Cc: Daniel Vetter 
>>> > Cc: David Weinehall 
>>> > Cc: Ville Syrjälä 
>>> > Signed-off-by: Jani Nikula 
>>> >
>>> > ---
>>> >
>>> > There was another patch from David increasing the size of the struct
>>> > [1], but that then regresses sdvo mapping parsing. It's the simplest to
>>> > just revert and fix this up properly for v4.3.
>>> >
>>> > [1] http://mid.gmane.org/20150813131415.GO6150@boom
>>> > ---
>>> >  drivers/gpu/drm/i915/intel_bios.c | 27 ---
>>> >  1 file changed, 4 insertions(+), 23 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>>> > b/drivers/gpu/drm/i915/intel_bios.c
>>> > index 3dcd59e694db..198fc3c3291b 100644
>>> > --- a/drivers/gpu/drm/i915/intel_bios.c
>>> > +++ b/drivers/gpu/drm/i915/intel_bios.c
>>> > @@ -1075,34 +1075,15 @@ parse_device_mapping(struct drm_i915_private 
>>> > *dev_priv,
>>> > const union child_device_config *p_child;
>>> > union child_device_config *child_dev_ptr;
>>> > int i, child_device_num, count;
>>> > -   u8 expected_size;
>>> > -   u16 block_size;
>>> > +   u16 block_size;
>>> >
>>> > p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
>>> > if (!p_defs) {
>>> > DRM_DEBUG_KMS("No general definition block is found, no 
>>> > devices defined.\n");
>>> > return;
>>> > }
>>> > -   if (bdb->version < 195) {
>>> > -   expected_size = 33;
>>> > -   } else if (bdb->version == 195) {
>>> > -   expected_size = 37;
>>> > -   } else if (bdb->version <= 197) {
>>> > -   expected_size = 38;
>>> > -   } else {
>>> > -   expected_size = 38;
>>> > -   DRM_DEBUG_DRIVER("Expected child_device_config size for 
>>> > BDB version %u not known; assuming %u\n",
>>> > -expected_size, bdb->version);
>>> > -   }
>>> > -
>>> > -   if (expected_size > sizeof(*p_child)) {
>>> > -   DRM_ERROR("child_device_config cannot fit in p_child\n");
>>> > -   return;
>>> > -   }
>>> > -
>>> > -   if (p_defs->child_dev_size != expected_size) {
>>> > -   DRM_ERROR("Size mismatch; child_device_config size=%u 
>>> > (expected %u); bdb->version: %u\n",
>>> > - p_defs->child_dev_size, expected_size, 
>>> > bdb->version);
>>> > +   if (p_defs->child_dev_size < sizeof(*p_child)) {
>>> > +   DRM_ERROR("General definiton block child device size is 
>>> > too small.\n");
>>> > return;
>>> > }
>>> > /* get 

Re: [Intel-gfx] [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping

[Jani Nikula]

On Tue, 18 Aug 2015, Ville Syrjälä  wrote:
> On Tue, Aug 18, 2015 at 02:28:55PM +0300, Jani Nikula wrote:
>> commit 75067ddecf21271631bc018d2fb23ddd09b66aae
>> Author: Antti Koskipaa 
>> Date:   Fri Jul 10 14:10:55 2015 +0300
>> 
>> drm/i915: Per-DDI I_boost override
>> 
>> increased size of union child_device_config without taking into account
>> the size check in parse_sdvo_device_mapping(). Switch the function over
>> to using the legacy struct only.
>> 
>> Fixes: 75067ddecf21 ("drm/i915: Per-DDI I_boost override")
>> Cc: Antti Koskipaa 
>> Cc: David Weinehall 
>> Signed-off-by: Jani Nikula 
>
> Reviewed-by: Ville Syrjälä 
>
> Also gave it a spin on my 946gz so:
> Tested-by: Ville Syrjälä 

Pushed to drm-intel-next-fixes, thanks for the review and testing.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/intel_bios.c | 50 
>> +++
>>  1 file changed, 25 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 8e46149bafdd..1b7e1a591a37 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -401,7 +401,7 @@ parse_sdvo_device_mapping(struct drm_i915_private 
>> *dev_priv,
>>  {
>>  struct sdvo_device_mapping *p_mapping;
>>  const struct bdb_general_definitions *p_defs;
>> -const union child_device_config *p_child;
>> +const struct old_child_dev_config *child; /* legacy */
>>  int i, child_device_num, count;
>>  u16 block_size;
>>  
>> @@ -410,14 +410,14 @@ parse_sdvo_device_mapping(struct drm_i915_private 
>> *dev_priv,
>>  DRM_DEBUG_KMS("No general definition block is found, unable to 
>> construct sdvo mapping.\n");
>>  return;
>>  }
>> -/* judge whether the size of child device meets the requirements.
>> - * If the child device size obtained from general definition block
>> - * is different with sizeof(struct child_device_config), skip the
>> - * parsing of sdvo device info
>> +
>> +/*
>> + * Only parse SDVO mappings when the general definitions block child
>> + * device size matches that of the *legacy* child device config
>> + * struct. Thus, SDVO mapping will be skipped for newer VBT.
>>   */
>> -if (p_defs->child_dev_size != sizeof(*p_child)) {
>> -/* different child dev size . Ignore it */
>> -DRM_DEBUG_KMS("different child size is found. Invalid.\n");
>> +if (p_defs->child_dev_size != sizeof(*child)) {
>> +DRM_DEBUG_KMS("Unsupported child device size for SDVO 
>> mapping.\n");
>>  return;
>>  }
>>  /* get the block size of general definitions */
>> @@ -427,37 +427,37 @@ parse_sdvo_device_mapping(struct drm_i915_private 
>> *dev_priv,
>>  p_defs->child_dev_size;
>>  count = 0;
>>  for (i = 0; i < child_device_num; i++) {
>> -p_child = child_device_ptr(p_defs, i);
>> -if (!p_child->old.device_type) {
>> +child = &child_device_ptr(p_defs, i)->old;
>> +if (!child->device_type) {
>>  /* skip the device block if device type is invalid */
>>  continue;
>>  }
>> -if (p_child->old.slave_addr != SLAVE_ADDR1 &&
>> -p_child->old.slave_addr != SLAVE_ADDR2) {
>> +if (child->slave_addr != SLAVE_ADDR1 &&
>> +child->slave_addr != SLAVE_ADDR2) {
>>  /*
>>   * If the slave address is neither 0x70 nor 0x72,
>>   * it is not a SDVO device. Skip it.
>>   */
>>  continue;
>>  }
>> -if (p_child->old.dvo_port != DEVICE_PORT_DVOB &&
>> -p_child->old.dvo_port != DEVICE_PORT_DVOC) {
>> +if (child->dvo_port != DEVICE_PORT_DVOB &&
>> +child->dvo_port != DEVICE_PORT_DVOC) {
>>  /* skip the incorrect SDVO port */
>>  DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
>>  continue;
>>  }
>>  DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
>> -" %s port\n",
>> -p_child->old.slave_addr,
>> -(p_child->old.dvo_port == DEVICE_PORT_DVOB) ?
>> -"SDVOB" : "SDVOC");
>> -p_mapping = &(dev_priv->sdvo_mappings[p_child->old.dvo_port - 
>> 1]);
>> +  " %s port\n",
>> +  child->slave_addr,
>> +  (child->dvo_port == DEVICE_PORT_DVOB) ?
>> +  "SDVOB" : "SDVOC");
>> +p_mapping = &(dev_priv->sdvo_mappings[child->dvo_port - 1]);
>>  if (!p_mapping->initialized) {
>> -p_mapping->dvo_port = p_child->old.dvo_port;
>> - 

Re: [Intel-gfx] [PATCH for v4.2] Revert "drm/i915: Allow parsing of variable size child device entries from VBT"

[Jani Nikula]

On Wed, 19 Aug 2015, Jani Nikula  wrote:
> On Wed, 19 Aug 2015, Daniel Vetter  wrote:
>> On Tue, Aug 18, 2015 at 10:00 AM, Ville Syrjälä
>>  wrote:
>>> On Tue, Aug 18, 2015 at 09:58:57AM -0700, Daniel Vetter wrote:
 On Tue, Aug 18, 2015 at 2:33 AM, Jani Nikula  wrote:
 > This reverts
 >
 > commit 047fe6e6db9161e69271f56daaafdaf2add023b1
 > Author: David Weinehall 
 > Date:   Tue Aug 4 16:55:52 2015 +0300
 >
 > drm/i915: Allow parsing of variable size child device entries from 
 > VBT
 >
 > That commit is not valid for v4.2, however it will be valid for v4.3. It
 > was simply queued too early.

 Nope, this patch from David also incidentally fixes a regression from
 90e4f1592bb6e82f6690f0e05a8aadc which is why I merged it to.
>>>
>>> What regression?
>>
>> Quoting the commit message: "...  since we're hitting a DRM_ERROR on
>> older platforms with this." Not every platform has the BSW vbt layout
>> ;-) Oh and why do I even bother to write this stuff when no one reads
>> it?
>
> I read it, and I think it's wrong. I even explained why in my commit
> message (yes, please read it). I don't think there was a DRM_ERROR on
> older platform with Ville's patch, on v4.2 where the revert is targeted,
> and even if there were, David's patch would *not* fix it. Indeed there
> was no discussion of a regression on the mailing list.
>
> If there was any regression, it was introduced by
>
> commit 75067ddecf21271631bc018d2fb23ddd09b66aae
> Author: Antti Koskipaa 
> Date:   Fri Jul 10 14:10:55 2015 +0300
>
> drm/i915: Per-DDI I_boost override
>
> when the size of union child_device_config changed. But that's headed
> for v4.3. And we're talking about v4.2, which is getting pretty
> urgent. We'll have a bit more time to sort out the mess that will land
> in v4.3 (and I've already sent one patch sorting out SDVO breakage).

I'm not waiting with this. I'm taking my chances, pushed to
drm-intel-fixes. Thanks for the review.

Note that the commit remains in drm-intel-next-fixes and
drm-intel-next-queued, but not in drm-intel-nightly due to this
revert. We'll need to clear that up, but for the time being getting v4.2
fixed up is of a higher priority to me.

BR,
Jani.




>
> BR,
> Jani.
>
>
>
>> -Daniel
>>
 If you
 want to revert this, you need to revert more (and with that also break
 BSW).

 Isn't there a more minimal fix we can do for 4.2?
 -Daniel

 >
 > The referenced regressing commit is just fine until the size of struct
 > common_child_dev_config changes, and that won't happen until
 > v4.3. Indeed, the expected size checks here rely on the increased size
 > of the struct, breaking new platforms.
 >
 > Fixes: 047fe6e6db91 ("drm/i915: Allow parsing of variable size child 
 > device entries from VBT")
 > Cc: Daniel Vetter 
 > Cc: David Weinehall 
 > Cc: Ville Syrjälä 
 > Signed-off-by: Jani Nikula 
 >
 > ---
 >
 > There was another patch from David increasing the size of the struct
 > [1], but that then regresses sdvo mapping parsing. It's the simplest to
 > just revert and fix this up properly for v4.3.
 >
 > [1] http://mid.gmane.org/20150813131415.GO6150@boom
 > ---
 >  drivers/gpu/drm/i915/intel_bios.c | 27 ---
 >  1 file changed, 4 insertions(+), 23 deletions(-)
 >
 > diff --git a/drivers/gpu/drm/i915/intel_bios.c 
 > b/drivers/gpu/drm/i915/intel_bios.c
 > index 3dcd59e694db..198fc3c3291b 100644
 > --- a/drivers/gpu/drm/i915/intel_bios.c
 > +++ b/drivers/gpu/drm/i915/intel_bios.c
 > @@ -1075,34 +1075,15 @@ parse_device_mapping(struct drm_i915_private 
 > *dev_priv,
 > const union child_device_config *p_child;
 > union child_device_config *child_dev_ptr;
 > int i, child_device_num, count;
 > -   u8 expected_size;
 > -   u16 block_size;
 > +   u16 block_size;
 >
 > p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
 > if (!p_defs) {
 > DRM_DEBUG_KMS("No general definition block is found, no 
 > devices defined.\n");
 > return;
 > }
 > -   if (bdb->version < 195) {
 > -   expected_size = 33;
 > -   } else if (bdb->version == 195) {
 > -   expected_size = 37;
 > -   } else if (bdb->version <= 197) {
 > -   expected_size = 38;
 > -   } else {
 > -   expected_size = 38;
 > -   DRM_DEBUG_DRIVER("Expected child_device_config size for 
 > BDB version %u not known; assuming %u\n",
 > -expected_size, bdb->version);
 > -   }
 > -
 > -   if (expected_size > sizeof(*p_child)) {
 > -   DRM_ERROR("child_device_config cannot fit in p_child\n");
 > -