[Intel-gfx] [PATCH v5 1/4] drm/i915: Add audio sync_audio_rate callback
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
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
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
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
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"
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
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"
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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"
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"
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"
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
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
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
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"
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
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
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
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
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"
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
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"
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"); > -