[Intel-gfx] [PATCH 1/4] drm/i915: Adding intel_panel_scale_none() helper function
From: dell
Signed-off-by: Xiong Zhang
---
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_panel.c | 10 ++
2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47cef0e..f57a0b4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1287,6 +1287,7 @@ int intel_panel_init(struct intel_panel *panel,
void intel_panel_fini(struct intel_panel *panel);
void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode);
+bool intel_panel_scale_none(struct intel_panel *panel);
void intel_pch_panel_fitting(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config,
int fitting_mode);
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index e2ab3f6..4a573ac 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -46,6 +46,16 @@ intel_fixed_panel_mode(const struct drm_display_mode
*fixed_mode,
drm_mode_set_crtcinfo(adjusted_mode, 0);
}
+bool
+intel_panel_scale_none(struct intel_panel *panel)
+{
+ if (panel->fitting_mode == DRM_MODE_SCALE_NONE ||
+ panel->fixed_mode == NULL)
+ return true;
+ else
+ return false;
+}
+
/**
* intel_find_panel_downclock - find the reduced downclock for LVDS in EDID
* @dev: drm device
--
1.8.2.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Adding Panel Filter function for DP
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);
+ }
+
if (ret)
return ret;
}
@@ -4688,15 +4705,14 @@ intel_dp_set_property(struct drm_connector *connector,
goto done;
}
- if (is_edp(intel_dp) &&
- property == connector->dev->mode_config.scaling_mode_property) {
- if (val == DRM_MODE_SCALE_NONE) {
- DRM_DEBUG_KMS("no scaling not supported\n");
+ if (property == connector->dev->mode_config.scaling_mode_property) {
+ if (is_edp(intel_dp) && val == DRM_MODE_SCALE_NONE) {
+ DRM_DEBUG_KMS("eDP: no scaling not supported\n");
return -EINVAL;
}
if (intel_connector->panel.fitting_mode == val) {
- /* the eDP scaling property is not changed */
+ /* the connector scaling property is not changed */
return 0;
}
intel_connector->panel.fitting_mode = val;
@@ -4989,13 +5005,22 @@ intel_dp_add_properties(struct intel_dp *intel_dp,
struct drm_connector *connect
intel_attach_broadcast_rgb_property(connector);
intel_dp->color_range_auto = true;
- if (is_edp(intel_dp)) {
+ /* Each pipe has panel filter since Ironlake. */
+ if (INTEL_INFO(connector->dev)->gen >= 5) {
drm_mode_create_scaling_mode_property(connector->dev);
- drm_object_attach_property(
- &connector->base,
- connector->dev->mode_config.scaling_mode_property,
- DRM_MODE_SCALE_ASPECT);
- intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
+ if (is_edp(intel_dp)) {
+ drm_object_attach_property(
+ &connector->base,
+
connector->dev->mode_config.scaling_mode_property,
+ DRM_MODE_SCALE_ASPECT);
+
[Intel-gfx] [PATCH 4/4] drm/i915: Adding panel filter for VGA
Signed-off-by: Xiong Zhang
---
drivers/gpu/drm/i915/intel_crt.c | 81 ++--
1 file changed, 77 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 521af2c..f8eedfc 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -288,24 +288,37 @@ intel_crt_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
{
struct drm_device *dev = connector->dev;
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+ struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
+ int max_clock = 0, clock;
- int max_clock = 0;
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
return MODE_NO_DBLESCAN;
- if (mode->clock < 25000)
+ clock = mode->clock;
+ if (!intel_panel_scale_none(&intel_connector->panel)) {
+ if (mode->hdisplay > fixed_mode->hdisplay)
+ return MODE_PANEL;
+
+ if (mode->vdisplay > fixed_mode->vdisplay)
+ return MODE_PANEL;
+
+ clock = fixed_mode->clock;
+ }
+
+ if (clock < 25000)
return MODE_CLOCK_LOW;
if (IS_GEN2(dev))
max_clock = 35;
else
max_clock = 40;
- if (mode->clock > max_clock)
+ if (clock > max_clock)
return MODE_CLOCK_HIGH;
/* The FDI receiver on LPT only supports 8bpc and only has 2 lanes. */
if (HAS_PCH_LPT(dev) &&
- (ironlake_get_lanes_required(mode->clock, 27, 24) > 2))
+ (ironlake_get_lanes_required(clock, 27, 24) > 2))
return MODE_CLOCK_HIGH;
return MODE_OK;
@@ -315,6 +328,22 @@ static bool intel_crt_compute_config(struct intel_encoder
*encoder,
struct intel_crtc_state *pipe_config)
{
struct drm_device *dev = encoder->base.dev;
+ struct drm_display_mode *adjusted_mode =
&pipe_config->base.adjusted_mode;
+ struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
+ struct intel_crt *intel_crt = intel_encoder_to_crt(encoder);
+ struct intel_connector *intel_connector = intel_crt->connector;
+
+ if (!intel_panel_scale_none(&intel_connector->panel)) {
+ intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
+ adjusted_mode);
+
+ if (!HAS_PCH_SPLIT(dev))
+ intel_gmch_panel_fitting(intel_crtc, pipe_config,
+intel_connector->panel.fitting_mode);
+ else
+ intel_pch_panel_fitting(intel_crtc, pipe_config,
+ intel_connector->panel.fitting_mode);
+ }
if (HAS_PCH_SPLIT(dev))
pipe_config->has_pch_encoder = true;
@@ -489,6 +518,7 @@ static struct edid *intel_crt_get_edid(struct drm_connector
*connector,
static int intel_crt_ddc_get_modes(struct drm_connector *connector,
struct i2c_adapter *adapter)
{
+ struct intel_connector *intel_connector = to_intel_connector(connector);
struct edid *edid;
int ret;
@@ -499,6 +529,20 @@ static int intel_crt_ddc_get_modes(struct drm_connector
*connector,
ret = intel_connector_update_modes(connector, edid);
kfree(edid);
+ if (ret && intel_connector->panel.fixed_mode == NULL) {
+ struct drm_display_mode *fixed_mode = NULL, *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);
+ }
+
return ret;
}
@@ -768,6 +812,18 @@ static int intel_crt_set_property(struct drm_connector
*connector,
struct drm_property *property,
uint64_t value)
{
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+
+ if (property == connector->dev->mode_config.scaling_mode_property) {
+ if (intel_connector->panel.fitting_mode == value)
+ return 0;
+
+ intel_connector->panel.fitting_mode = value;
+
+ if (connector->encoder && connector->encoder->crtc)
+ intel_crtc_restore_mode(connector->encoder->crtc);
+ }
+
return 0;
}
@@ -844,6 +900,21 @@ static const struct dmi_system_id intel_no_crt[] = {
{ }
};
+static voi
[Intel-gfx] [PATCH 3/4] drm/i915: Adding panel filter for HDMI
Signed-off-by: Xiong Zhang
---
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_hdmi.c | 79 ---
2 files changed, 76 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f57a0b4..0a4632b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -672,6 +672,8 @@ struct intel_hdmi {
bool enable,
struct drm_display_mode *adjusted_mode);
bool (*infoframe_enabled)(struct drm_encoder *encoder);
+
+ struct intel_connector *attached_connector;
};
struct intel_dp_mst_encoder;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 70bad5b..6f1638c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1191,6 +1191,8 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
{
struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
struct drm_device *dev = intel_hdmi_to_dev(hdmi);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+ struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
enum drm_mode_status status;
int clock;
@@ -1201,6 +1203,18 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
clock *= 2;
+ if (!intel_panel_scale_none(&intel_connector->panel)) {
+ if (mode->hdisplay > fixed_mode->hdisplay)
+ return MODE_PANEL;
+
+ if (mode->vdisplay > fixed_mode->vdisplay)
+ return MODE_PANEL;
+
+ clock = fixed_mode->clock;
+ if (fixed_mode->flags & DRM_MODE_FLAG_DBLCLK)
+ clock *= 2;
+ }
+
/* check if we can do 8bpc */
status = hdmi_port_clock_valid(hdmi, clock, true);
@@ -1249,8 +1263,9 @@ bool intel_hdmi_compute_config(struct intel_encoder
*encoder,
struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
struct drm_device *dev = encoder->base.dev;
struct drm_display_mode *adjusted_mode =
&pipe_config->base.adjusted_mode;
- int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
- int clock_12bpc = clock_8bpc * 3 / 2;
+ struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
+ struct intel_connector *intel_connector =
intel_hdmi->attached_connector;
+ int clock_8bpc, clock_12bpc;
int desired_bpp;
pipe_config->has_hdmi_sink = intel_hdmi->has_hdmi_sink;
@@ -1258,6 +1273,18 @@ bool intel_hdmi_compute_config(struct intel_encoder
*encoder,
if (pipe_config->has_hdmi_sink)
pipe_config->has_infoframe = true;
+ if (!intel_panel_scale_none(&intel_connector->panel)) {
+ intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
+ adjusted_mode);
+
+ if (!HAS_PCH_SPLIT(dev))
+ intel_gmch_panel_fitting(intel_crtc, pipe_config,
+intel_connector->panel.fitting_mode);
+ else
+ intel_pch_panel_fitting(intel_crtc, pipe_config,
+intel_connector->panel.fitting_mode);
+ }
+
if (intel_hdmi->color_range_auto) {
/* See CEA-861-E - 5.1 Default Encoding Parameters */
if (pipe_config->has_hdmi_sink &&
@@ -1267,6 +1294,8 @@ bool intel_hdmi_compute_config(struct intel_encoder
*encoder,
intel_hdmi->color_range = 0;
}
+ clock_8bpc = adjusted_mode->crtc_clock;
+ clock_12bpc = clock_8bpc * 3 / 2;
if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) {
pipe_config->pixel_multiplier = 2;
clock_8bpc *= 2;
@@ -1410,13 +1439,30 @@ intel_hdmi_force(struct drm_connector *connector)
static int intel_hdmi_get_modes(struct drm_connector *connector)
{
- struct edid *edid;
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+ struct edid *edid = intel_connector->detect_edid;
+ int ret;
- edid = to_intel_connector(connector)->detect_edid;
if (edid == NULL)
return 0;
- return intel_connector_update_modes(connector, edid);
+ ret = intel_connector_update_modes(connector, edid);
+ if (ret && intel_connector->panel.fixed_mode == NULL) {
+ 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,
+
[Intel-gfx] [PATCH 1/4] drm/i915: Adding intel_panel_scale_none() helper function
Signed-off-by: Xiong Zhang
---
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_panel.c | 10 ++
2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47cef0e..f57a0b4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1287,6 +1287,7 @@ int intel_panel_init(struct intel_panel *panel,
void intel_panel_fini(struct intel_panel *panel);
void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode);
+bool intel_panel_scale_none(struct intel_panel *panel);
void intel_pch_panel_fitting(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config,
int fitting_mode);
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index e2ab3f6..4a573ac 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -46,6 +46,16 @@ intel_fixed_panel_mode(const struct drm_display_mode
*fixed_mode,
drm_mode_set_crtcinfo(adjusted_mode, 0);
}
+bool
+intel_panel_scale_none(struct intel_panel *panel)
+{
+ if (panel->fitting_mode == DRM_MODE_SCALE_NONE ||
+ panel->fixed_mode == NULL)
+ return true;
+ else
+ return false;
+}
+
/**
* intel_find_panel_downclock - find the reduced downclock for LVDS in EDID
* @dev: drm device
--
1.8.2.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts callback
From: Libin Yang
Add the set_ncts callback.
With the callback, audio driver can trigger
i915 driver to set the proper N/CTS
based on different sample rates.
Signed-off-by: Libin Yang
---
include/drm/i915_component.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index c9a8b64..7305881 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -33,6 +33,8 @@ struct i915_audio_component {
void (*put_power)(struct device *);
void (*codec_wake_override)(struct device *, bool enable);
int (*get_cdclk_freq)(struct device *);
+ int (*set_ncts)(struct device *, int port, int dev_entry,
+ 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 v4 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 value, the N/CTS need to be set manually if audio
is playing.
Signed-off-by: Libin Yang
---
drivers/gpu/drm/i915/i915_reg.h| 6 ++
drivers/gpu/drm/i915/intel_audio.c | 42 ++
2 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index da2d128..85f3beb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7030,6 +7030,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 eddf37f..082f96d 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -235,6 +235,9 @@ static void hsw_audio_codec_enable(struct drm_connector
*connector,
const uint8_t *eld = connector->eld;
uint32_t tmp;
int len, i;
+ int cvt_idx;
+ int n_low, n_up, n;
+ int base_rate, mul, div, rate;
DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
pipe_name(pipe), drm_eld_size(eld));
@@ -267,6 +270,21 @@ static void hsw_audio_codec_enable(struct drm_connector
*connector,
tmp |= AUDIO_ELD_VALID(pipe);
I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
+ if ((mode->clock == 297000) ||
+ (mode->clock == DIV_ROUND_UP(297000 * 1000, 1001))) {
+ 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;
+ }
+
/* Enable timestamps */
tmp = I915_READ(HSW_AUD_CFG(pipe));
tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
@@ -276,6 +294,30 @@ static void hsw_audio_codec_enable(struct drm_connector
*connector,
tmp |= AUD_CONFIG_N_VALUE_INDEX;
else
tmp |= audio_config_hdmi_pixel_clock(mode);
+
+ if ((mode->clock != 297000) &&
+ (mode->clock != DIV_ROUND_UP(297000 * 1000, 1001))) {
+ tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+ } else {
+ for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
+ if ((rate == aud_ncts[i].sample_rate) &&
+ (mode->clock == aud_ncts[i].clock)) {
+ n = aud_ncts[i].n;
+ break;
+ }
+ }
+ if (n != 0) {
+ tmp |= AUD_CONFIG_N_PROG_ENABLE;
+ n_low = n & 0xfff;
+ n_up = (n >> 12) & 0xff;
+ tmp |= AUD_CONFIG_N_PROG_ENABLE;
+ tmp &= ~AUD_CONFIG_UPPER_N_MASK;
+ tmp |= (n_up << AUD_CONFIG_UPPER_N_SHIFT);
+ tmp &= ~AUD_CONFIG_LOWER_N_MASK;
+ tmp |= (n_low << AUD_CONFIG_LOWER_N_SHIFT);
+ }
+ }
+
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
[Intel-gfx] [PATCH v2 2/4] drm/i915: implement set_ncts callback
From: Libin Yang
Display 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/i915_reg.h| 2 +
drivers/gpu/drm/i915/intel_audio.c | 95 ++
2 files changed, 97 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ea46d68..da2d128 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7014,6 +7014,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, \
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index dc32cf4..eddf37f 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -68,6 +68,30 @@ static const struct {
{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
};
+#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)
{
@@ -514,12 +538,83 @@ static int i915_audio_component_get_cdclk_freq(struct
device *dev)
return ret;
}
+static int i915_audio_component_set_ncts(struct device *dev, int port,
+ int dev_entry, 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 i;
+ int n_low, n_up, n = 0;
+
+ /* 1. get the pipe */
+ for_each_intel_encoder(drm_dev, intel_encoder) {
+ 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 == -1) {
+ 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. Need set the N/CTS manually at some frequencies */
+ if ((mode->clock != TMDS_297M) &&
+ (mode->clock != TMDS_296M)) {
+ tmp = I915_READ(HSW_AUD_CFG(pipe));
+ tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+ I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+ return 0;
+ }
+
+ /* 3. calculate the N/CTS */
+ for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
+ if ((rate == aud_ncts[i].sample_rate) &&
+ (mode->clock == aud_ncts[i].clock)) {
+ n = aud_ncts[i].n;
+ break;
+ }
+ }
+ if (n == 0)
+ return 0;
+ n_low = n & 0xfff;
+ n_up = (n >> 12) & 0xff;
+
+ /* 4. set the N/CTS */
+ tmp = I915_READ(HSW_AUD_CFG(pipe));
+ tmp |= AUD_CONFIG_N_PROG_ENABLE;
+ tmp &= ~AUD_CONFIG_UPPER_N_MASK;
+ tmp |= (n_up << AUD_CONFIG_UPPER_N_SHIFT);
+ tmp &= ~AUD_CONFIG_LOWER_N_MASK;
+ tmp |= (n_low << AUD_CONFIG_LOWER_N_SHIFT);
+ I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+
+ return 0;
+}
+
static const struct i915_audio_component_ops i915_audio_component_ops = {
.owner
[Intel-gfx] [PATCH v3 3/4] ALSA: hda - display audio call ncts callback
From: Libin Yang
On some Intel platforms, display audio need set N/CTS
manually at some TMDS frequencies.
Signed-off-by: Libin Yang
---
sound/pci/hda/patch_hdmi.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index a97db5f..918435e 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,17 @@ static int generic_hdmi_playback_pcm_prepare(struct
hda_pcm_stream *hinfo,
intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx);
}
+ /* Set N/CTS for HSW, BDW and SKL platforms.
+* These platforms need call set_ncts to set the N/CTS manually,
+* otherwise there is no sound in some sample rates.
+*/
+ if (is_haswell_plus(codec)) {
+ /* Todo: add DP1.2 MST audio support later */
+ if (acomp && acomp->ops && acomp->ops->set_ncts)
+ acomp->ops->set_ncts(acomp->dev,
+ intel_pin2port(pin_nid),
+ 0, 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
Re: [Intel-gfx] [PATCH] drm/i915: Clean up lrc context init
On 07/08/2015 11:13, Chris Wilson wrote:
On Fri, Aug 07, 2015 at 11:05:24AM +0100, Nick Hoath wrote:
Clean up lrc context init by:
- 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.
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 923a3c4..37b440a 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,14 +1010,47 @@ 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);
}
+
+ if (ring->id == RCS && !ctx->rcs_initialized) {
+ if (ring->init_context) {
+ struct drm_i915_gem_request *req;
+
+ ret = i915_gem_request_alloc(ring,
+ ring->default_context, &req);
+ if (ret) {
+ DRM_ERROR("ring create req: %d\n",
+ ret);
+ i915_gem_request_cancel(req);
+ goto validate_error;
+ }
+
+ ret = ring->init_context(req);
+ if (ret) {
+ DRM_ERROR("ring init context: %d\n",
+ ret);
+ i915_gem_request_cancel(req);
+ goto validate_error;
+ }
+ i915_add_request_no_flush(req);
+ }
+
+ ctx->rcs_initialized = true;
+ }
}
Nak.
"Clean up"?
Try reading 4798
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH libdrm v3 1/2] intel: Add EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag
On Fri, 2015-08-07 at 12:36 +0100, Michel Thierry wrote: > Hi Michał, > > Ben suggested having the set/clear in emit reloc as this is the only > place mesa cares about these wa. > > But I see your point, this will be used not only by mesa, so we > should > have something that is good for all the other projects. > > -Michel If there are no comments from anyone else then I'm fine with public API from last version. But comments about the error handling and implementation details could still be addressed. -Michał ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
On Mon, 10 Aug 2015 09:32:11 +0200,
libin.y...@intel.com wrote:
>
> From: Libin Yang
>
> When modeset occurs and the TMDS frequency is set to some
> speical value, the N/CTS need to be set manually if audio
> is playing.
>
> Signed-off-by: Libin Yang
> ---
> drivers/gpu/drm/i915/i915_reg.h| 6 ++
> drivers/gpu/drm/i915/intel_audio.c | 42
> ++
> 2 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index da2d128..85f3beb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7030,6 +7030,12 @@ enum skl_disp_power_wells {
> _HSW_AUD_DIG_CNVT_2)
> #define DIP_PORT_SEL_MASK0x3
>
> +#define _HSW_AUD_STR_DESC_1 0x65084
> +#define _HSW_AUD_STR_DESC_2 0x65184
> +#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 eddf37f..082f96d 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -235,6 +235,9 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> const uint8_t *eld = connector->eld;
> uint32_t tmp;
> int len, i;
> + int cvt_idx;
> + int n_low, n_up, n;
> + int base_rate, mul, div, rate;
>
> DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
> pipe_name(pipe), drm_eld_size(eld));
> @@ -267,6 +270,21 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> tmp |= AUDIO_ELD_VALID(pipe);
> I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
>
> + if ((mode->clock == 297000) ||
> + (mode->clock == DIV_ROUND_UP(297000 * 1000, 1001))) {
TMDS_297M and TMDS_296M can be used here?
> + 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;
> + }
> +
> /* Enable timestamps */
> tmp = I915_READ(HSW_AUD_CFG(pipe));
> tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> @@ -276,6 +294,30 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> tmp |= AUD_CONFIG_N_VALUE_INDEX;
> else
> tmp |= audio_config_hdmi_pixel_clock(mode);
> +
> + if ((mode->clock != 297000) &&
> + (mode->clock != DIV_ROUND_UP(297000 * 1000, 1001))) {
> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> + } else {
> + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> + if ((rate == aud_ncts[i].sample_rate) &&
> + (mode->clock == aud_ncts[i].clock)) {
> + n = aud_ncts[i].n;
> + break;
> + }
> + }
Missing initialization of n?
> + if (n != 0) {
> + tmp |= AUD_CONFIG_N_PROG_ENABLE;
> + n_low = n & 0xfff;
> + n_up = (n >> 12) & 0xff;
> + tmp |= AUD_CONFIG_N_PROG_ENABLE;
You don't need to set it twice.
> + tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> + tmp |= (n_up << AUD_CONFIG_UPPER_N_SHIFT);
> + tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> + tmp |= (n_low << AUD_CONFIG_LOWER_N_SHIFT);
> + }
> + }
> +
> I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> }
>
> --
> 1.9.1
>
thanks,
Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
On Mon, Aug 10, 2015 at 04:50:37AM +, Jindal, Sonika wrote: > Hi Daniel, > > That patch was already merged: > http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.html > > For SKL, the above patch helped in getting the correct ISR bits set. > One option is to enable the HDMI optimization from VLV onwards. > I don't have an ivb machine to try out the issue. ivb is simple the machine I have here, but when we tried this the last time around we had reports for all platforms (your patch still doesn't cite the relevant sha1 btw). I think there are 3 possible explanations: a) we do something wrong with hpd handling on these platforms. That seems to be the explanation you favour (with the gen >= 7 checks and all that), but I think it's very unlikely: On each platform where we had reports of hpd being broken there was also machines where hpd works perfectly fine. b) There's broken HDMI (or DVI) sinks out there. If that's the case we can never merge your patch. c) There's something in vbt or somewhere else that tells the windows driver whether using hpd or not is ok (and the hpd problem is actually an issue with certain OEM machines ...). I hoped that with your hpd handling fix we'd have some indication that our hpd troubles are of type a). But since I tested with your patch that didn't work out. And until we have some evidence that our hpd troubles aren't type b) I really don't want to merge any patch which relies upon hpd bits for hdmi. -Daniel > > Regards, > Sonika > > -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Friday, August 7, 2015 6:54 PM > To: Jindal, Sonika > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series > > On Tue, Jul 14, 2015 at 05:21:20PM +0530, Sonika Jindal wrote: > > This series adds some optimization related to reading of edid only > > when required and when live status says so. > > The comments in the patches explain more. > > This series breaks my funky ivb machine with the broken hpd bits: When I > unplug the screen the system never invalidates the edid and so never notices > that it's gone. > > Now iirc you've discovered an issue in our hpd irq handler which can cause > lost hpd bits, but that patch isn't in this series. And iirc I asked you to > resend everything since that hw fix also wasn't in the last series. And I > can't find it any more. Did I just dream about this other patch to fix hpd on > big core? > > Thanks, Daniel > > > v2: > >Some refactoring is with this series. > > > >Also, right now this is done for platforms gen7 and above because we > >couldn't test with older platforms. For newer platforms it works > >reliably. > > > >For HPD and live status to work on SKL, following patch is required: > >"drm/i915: Handle HPD when it has actually occurred" > > > > Shashank Sharma (2): > > drm/i915: add attached connector to hdmi container > > drm/i915: Add HDMI probe function > > > > Sonika Jindal (1): > > drm/i915: Check live status before reading edid > > > > drivers/gpu/drm/i915/intel_dp.c |4 +- > > drivers/gpu/drm/i915/intel_drv.h |3 ++ > > drivers/gpu/drm/i915/intel_hdmi.c | 93 > > + > > 3 files changed, 88 insertions(+), 12 deletions(-) > > > > -- > > 1.7.10.4 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation 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] drm/i915: Spam less on dp aux send/receive problems
On Thu, 06 Aug 2015, Mika Kuoppala wrote:
> If we encounter frequent problems with dp aux channel
> communications, we end up spamming the dmesg with the
> exact similar trace and status.
>
> Inject a new backtrace only if we have new information
> to share as otherwise we flush out all other important
> stuff.
>
> Signed-off-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/intel_dp.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index df7e2cf..6b2f7af 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -849,8 +849,15 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> }
>
> if (try == 3) {
> - WARN(1, "dp_aux_ch not started status 0x%08x\n",
> - I915_READ(ch_ctl));
> + static u32 last_status = -1;
> + const u32 status = I915_READ(ch_ctl);
> +
> + if (status != last_status) {
> + WARN(1, "dp_aux_ch not started status 0x%08x\n",
> + status);
> + last_status = status;
> + }
> +
But now you'll also skip the logging even if there's been a day and a
million successful transfers since the last error... I understand your
concern, but if you feel you must do something like this, please at
least reset last_status on success.
BR,
Jani.
> ret = -EBUSY;
> goto out;
> }
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
> -Original Message-
> From: Takashi Iwai [mailto:ti...@suse.de]
> Sent: Monday, August 10, 2015 4:04 PM
> To: Yang, Libin
> Cc: alsa-de...@alsa-project.org; intel-gfx@lists.freedesktop.org;
> daniel.vet...@ffwll.ch
> Subject: Re: [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
>
> On Mon, 10 Aug 2015 09:32:11 +0200,
> libin.y...@intel.com wrote:
> >
> > From: Libin Yang
> >
> > When modeset occurs and the TMDS frequency is set to some
> > speical value, the N/CTS need to be set manually if audio
> > is playing.
> >
> > Signed-off-by: Libin Yang
> > ---
> > drivers/gpu/drm/i915/i915_reg.h| 6 ++
> > drivers/gpu/drm/i915/intel_audio.c | 42
> ++
> > 2 files changed, 48 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > index da2d128..85f3beb 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7030,6 +7030,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 eddf37f..082f96d 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -235,6 +235,9 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> > const uint8_t *eld = connector->eld;
> > uint32_t tmp;
> > int len, i;
> > + int cvt_idx;
> > + int n_low, n_up, n;
> > + int base_rate, mul, div, rate;
> >
> > DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes
> ELD\n",
> > pipe_name(pipe), drm_eld_size(eld));
> > @@ -267,6 +270,21 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> > tmp |= AUDIO_ELD_VALID(pipe);
> > I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> >
> > + if ((mode->clock == 297000) ||
> > + (mode->clock == DIV_ROUND_UP(297000 * 1000, 1001)))
> {
>
> TMDS_297M and TMDS_296M can be used here?
Oh, sorry, I forgot to use it here.
>
>
> > + 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;
> > + }
> > +
> > /* Enable timestamps */
> > tmp = I915_READ(HSW_AUD_CFG(pipe));
> > tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > @@ -276,6 +294,30 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> > tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > else
> > tmp |= audio_config_hdmi_pixel_clock(mode);
> > +
> > + if ((mode->clock != 297000) &&
> > + (mode->clock != DIV_ROUND_UP(297000 * 1000, 1001)))
> {
> > + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > + } else {
> > + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> > + if ((rate == aud_ncts[i].sample_rate) &&
> > + (mode->clock == aud_ncts[i].clock)) {
> > + n = aud_ncts[i].n;
> > + break;
> > + }
> > + }
>
> Missing initialization of n?
Oh, yes, I will fix it.
>
> > + if (n != 0) {
> > + tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > + n_low = n & 0xfff;
> > + n_up = (n >> 12) & 0xff;
> > + tmp |= AUD_CONFIG_N_PROG_ENABLE;
>
> You don't need to set it twice.
Yes.
>
> > + tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > + tmp |= (n_up << AUD_CONFIG_UPPER_N_SHIFT);
> > + tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > + tmp |= (n_low <<
> AUD_CONFIG_LOWER_N_SHIFT);
> > + }
> > + }
> > +
> > I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > }
> >
> > --
> > 1.9.1
> >
>
>
> thanks,
>
> Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
On Fri, Aug 07, 2015 at 10:04:47AM -0300, Paulo Zanoni wrote: > 2015-08-06 18:33 GMT-03:00 Daniel Vetter : > > This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c. > > > > The point of testing for LAST_FLAG + 1 is to catch abi extensions - > > despite our best efforts we really suck at properly reviewing for test > > coverage when extending ABI. > > > > The real bug here is that David Weinhall hasn't submitted updated igts > > for the NO_ZEROMAP feature yet. Imo the right course of action is to > > revert that feature if the testcase don't show up within a few days. > > > > Cc: David Weinehall > > Cc: Jesse Barnes > > Signed-off-by: Daniel Vetter > > --- > > tests/gem_ctx_param_basic.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c > > index 5ff3b13f4c7a..b44b37cf0538 100644 > > --- a/tests/gem_ctx_param_basic.c > > +++ b/tests/gem_ctx_param_basic.c > > @@ -98,7 +98,7 @@ igt_main > > ctx_param.size = 0; > > } > > > > - ctx_param.param = -1; > > + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1; > > How about adding a comment somewhere "If this breaks it's because we > extended the number of params without updating IGT. Please add the > proper tests for the new param"? That will help preventing us from > making the same error again next year. Good idea! Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
On 8/10/2015 1:38 PM, Daniel Vetter wrote: On Mon, Aug 10, 2015 at 04:50:37AM +, Jindal, Sonika wrote: Hi Daniel, That patch was already merged: http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.html For SKL, the above patch helped in getting the correct ISR bits set. One option is to enable the HDMI optimization from VLV onwards. I don't have an ivb machine to try out the issue. ivb is simple the machine I have here, but when we tried this the last time around we had reports for all platforms (your patch still doesn't cite the relevant sha1 btw). I think there are 3 possible explanations: Yes, I don't know which were those patches and how to find them.. a) we do something wrong with hpd handling on these platforms. That seems to be the explanation you favour (with the gen >= 7 checks and all that), but I think it's very unlikely: On each platform where we had reports of hpd being broken there was also machines where hpd works perfectly fine. Not sure, I will find one ivb system and try on that. b) There's broken HDMI (or DVI) sinks out there. If that's the case we can never merge your patch. I doubt this because we have tested these patches with many sinks in the past with VLV/CHV. c) There's something in vbt or somewhere else that tells the windows driver whether using hpd or not is ok (and the hpd problem is actually an issue with certain OEM machines ...). No, nothing like that. I hoped that with your hpd handling fix we'd have some indication that our hpd troubles are of type a). But since I tested with your patch that didn't work out. And until we have some evidence that our hpd troubles aren't type b) I really don't want to merge any patch which relies upon hpd bits for hdmi. -Daniel I will try on ivb. Regards Sonika Regards, Sonika -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Friday, August 7, 2015 6:54 PM To: Jindal, Sonika Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series On Tue, Jul 14, 2015 at 05:21:20PM +0530, Sonika Jindal wrote: This series adds some optimization related to reading of edid only when required and when live status says so. The comments in the patches explain more. This series breaks my funky ivb machine with the broken hpd bits: When I unplug the screen the system never invalidates the edid and so never notices that it's gone. Now iirc you've discovered an issue in our hpd irq handler which can cause lost hpd bits, but that patch isn't in this series. And iirc I asked you to resend everything since that hw fix also wasn't in the last series. And I can't find it any more. Did I just dream about this other patch to fix hpd on big core? Thanks, Daniel v2: Some refactoring is with this series. Also, right now this is done for platforms gen7 and above because we couldn't test with older platforms. For newer platforms it works reliably. For HPD and live status to work on SKL, following patch is required: "drm/i915: Handle HPD when it has actually occurred" Shashank Sharma (2): drm/i915: add attached connector to hdmi container drm/i915: Add HDMI probe function Sonika Jindal (1): drm/i915: Check live status before reading edid drivers/gpu/drm/i915/intel_dp.c |4 +- drivers/gpu/drm/i915/intel_drv.h |3 ++ drivers/gpu/drm/i915/intel_hdmi.c | 93 + 3 files changed, 88 insertions(+), 12 deletions(-) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 1/3] drm/i915: Only update the current userptr worker
The userptr worker allows for a slight race condition where upon there
may two or more threads calling get_user_pages for the same object. When
we have the array of pages, then we serialise the update of the object.
However, the worker should only overwrite the obj->userptr.work pointer
if and only if it is the active one. Currently we clear it for a
secondary worker with the effect that we may rarely force a second
lookup.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_userptr.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index d11901d590ac..800a5394aa1e 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -571,25 +571,25 @@ __i915_gem_userptr_get_pages_worker(struct work_struct
*_work)
struct get_pages_work *work = container_of(_work, typeof(*work), work);
struct drm_i915_gem_object *obj = work->obj;
struct drm_device *dev = obj->base.dev;
- const int num_pages = obj->base.size >> PAGE_SHIFT;
+ const int npages = obj->base.size >> PAGE_SHIFT;
struct page **pvec;
int pinned, ret;
ret = -ENOMEM;
pinned = 0;
- pvec = kmalloc(num_pages*sizeof(struct page *),
+ pvec = kmalloc(npages*sizeof(struct page *),
GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
if (pvec == NULL)
- pvec = drm_malloc_ab(num_pages, sizeof(struct page *));
+ pvec = drm_malloc_ab(npages, sizeof(struct page *));
if (pvec != NULL) {
struct mm_struct *mm = obj->userptr.mm->mm;
down_read(&mm->mmap_sem);
- while (pinned < num_pages) {
+ while (pinned < npages) {
ret = get_user_pages(work->task, mm,
obj->userptr.ptr + pinned *
PAGE_SIZE,
-num_pages - pinned,
+npages - pinned,
!obj->userptr.read_only, 0,
pvec + pinned, NULL);
if (ret < 0)
@@ -601,20 +601,20 @@ __i915_gem_userptr_get_pages_worker(struct work_struct
*_work)
}
mutex_lock(&dev->struct_mutex);
- if (obj->userptr.work != &work->work) {
- ret = 0;
- } else if (pinned == num_pages) {
- ret = __i915_gem_userptr_set_pages(obj, pvec, num_pages);
- if (ret == 0) {
- list_add_tail(&obj->global_list,
&to_i915(dev)->mm.unbound_list);
- obj->get_page.sg = obj->pages->sgl;
- obj->get_page.last = 0;
-
- pinned = 0;
+ if (obj->userptr.work == &work->work) {
+ if (pinned == npages) {
+ ret = __i915_gem_userptr_set_pages(obj, pvec, npages);
+ if (ret == 0) {
+ list_add_tail(&obj->global_list,
+ &to_i915(dev)->mm.unbound_list);
+ obj->get_page.sg = obj->pages->sgl;
+ obj->get_page.last = 0;
+ pinned = 0;
+ }
}
+ obj->userptr.work = ERR_PTR(ret);
}
- obj->userptr.work = ERR_PTR(ret);
obj->userptr.workers--;
drm_gem_object_unreference(&obj->base);
mutex_unlock(&dev->struct_mutex);
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 2/3] drm/i915: Fix userptr deadlock with aliased GTT mmappings
Michał Winiarski found a really evil way to trigger a struct_mutex
deadlock with userptr. He found that if he allocated a userptr bo and
then GTT mmaped another bo, or even itself, at the same address as the
userptr using MAP_FIXED, he could then cause a deadlock any time we then
had to invalidate the GTT mmappings (so at will). Tvrtko then found by
repeatedly allocating GTT mmappings he could alias with an old userptr
mmap and also trigger the deadlock.
To counter act the deadlock, we make the observation that we only need
to take the struct_mutex if the object has any pages to revoke, and that
before userspace can alias with the userptr address space, it must have
invalidated the userptr->pages. Thus if we can check for those pages
outside of the struct_mutex, we can avoid the deadlock. To do so we
introduce a separate flag for userptr objects that we can inspect from
the mmu-notifier underneath its spinlock.
The patch makes one eye-catching change. That is the removal serial=0
after detecting a to-be-freed object inside the invalidate walker. I
felt setting serial=0 was a questionable pessimisation: it denies us the
chance to reuse the current iterator for the next loop (before it is
freed) and being explicit makes the reader question the validity of the
locking (since the object-free race could occur elsewhere). The
serialisation of the iterator is through the spinlock, if the object is
freed before the next loop then the notifier.serial will be incremented
and we start the walk from the beginning as we detect the invalid cache.
To try and tame the error paths and interactions with the userptr->active
flag, we have to do a fair amount of rearranging of get_pages_userptr().
v2: Grammar fixes
v3: Reorder set-active so that it is only set when obj->pages is set
(and so needs cancellation). Only the order of setting obj->pages and
the active-flag is crucial. Calling gup after invalidate-range begin
means the userptr sees the new set of backing storage (and so will not
need to invalidate its new pages), but we have to be careful not to set
the active-flag prior to successfully establishing obj->pages.
v4: Take the active->flag early so we know in the mmu-notifier when we
have to cancel a pending gup-worker.
Reported-by: Michał Winiarski
Testcase: igt/gem_userptr_blits/map-fixed*
Signed-off-by: Chris Wilson
Cc: Michał Winiarski
Cc: Tvrtko Ursulin
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/i915_gem_userptr.c | 111 ++--
1 file changed, 78 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 800a5394aa1e..e21f885db87b 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -59,6 +59,7 @@ struct i915_mmu_object {
struct interval_tree_node it;
struct list_head link;
struct drm_i915_gem_object *obj;
+ bool active;
bool is_linear;
};
@@ -114,7 +115,8 @@ restart:
obj = mo->obj;
- if (!kref_get_unless_zero(&obj->base.refcount))
+ if (!mo->active ||
+ !kref_get_unless_zero(&obj->base.refcount))
continue;
spin_unlock(&mn->lock);
@@ -151,7 +153,8 @@ static void
i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
else
it = interval_tree_iter_first(&mn->objects, start, end);
if (it != NULL) {
- obj = container_of(it, struct i915_mmu_object, it)->obj;
+ struct i915_mmu_object *mo =
+ container_of(it, struct i915_mmu_object, it);
/* The mmu_object is released late when destroying the
* GEM object so it is entirely possible to gain a
@@ -160,11 +163,9 @@ static void
i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
* the struct_mutex - and consequently use it after it
* is freed and then double free it.
*/
- if (!kref_get_unless_zero(&obj->base.refcount)) {
- spin_unlock(&mn->lock);
- serial = 0;
- continue;
- }
+ if (mo->active &&
+ kref_get_unless_zero(&mo->obj->base.refcount))
+ obj = mo->obj;
serial = mn->serial;
}
@@ -566,6 +567,30 @@ __i915_gem_userptr_set_pages(struct drm_i915_gem_object
*obj,
}
static void
+__i915_gem_userptr_set_active(struct drm_i915_gem_object *obj,
+ bool value)
+{
+ /* During mm_invalidate_range we need to cancel any userptr that
+* overlaps the range being invalidated. Doing so requires the
+
[Intel-gfx] [PATCH v3 3/3] drm/i915: Use a task to cancel the userptr on invalidate_range
Whilst discussing possible ways to trigger an invalidate_range on a
userptr with an aliased GGTT mmapping (and so cause a struct_mutex
deadlock), the conclusion is that we can, and we must, prevent any
possible deadlock by avoiding taking the mutex at all during
invalidate_range. This has numerous advantages all of which stem from
avoid the sleeping function from inside the unknown context. In
particular, it simplifies the invalidate_range because we no longer
have to juggle the spinlock/mutex and can just hold the spinlock
for the entire walk. To compensate, we have to make get_pages a bit more
complicated in order to serialise with a pending cancel_userptr worker.
As we hold the struct_mutex, we have no choice but to return EAGAIN and
hope that the worker is then flushed before we retry after reacquiring
the struct_mutex.
The important caveat is that the invalidate_range itself is no longer
synchronous. There exists a small but definite period in time in which
the old PTE's page remain accessible via the GPU. Note however that the
physical pages themselves are not invalidated by the mmu_notifier, just
the CPU view of the address space. The impact should be limited to a
delay in pages being flushed, rather than a possibility of writing to
the wrong pages. The only race condition that this worsens is remapping
an userptr active on the GPU where fresh work may still reference the
old pages due to struct_mutex contention. Given that userspace is racing
with the GPU, it is fair to say that the results are undefined.
v2: Only queue (and importantly only take one refcnt) the worker once.
Signed-off-by: Chris Wilson
Cc: Michał Winiarski
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_gem_userptr.c | 146 +---
1 file changed, 60 insertions(+), 86 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index e21f885db87b..17344dac807e 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -50,7 +50,6 @@ struct i915_mmu_notifier {
struct mmu_notifier mn;
struct rb_root objects;
struct list_head linear;
- unsigned long serial;
bool has_linear;
};
@@ -59,14 +58,16 @@ struct i915_mmu_object {
struct interval_tree_node it;
struct list_head link;
struct drm_i915_gem_object *obj;
+ struct work_struct work;
bool active;
bool is_linear;
};
-static unsigned long cancel_userptr(struct drm_i915_gem_object *obj)
+static void __cancel_userptr__worker(struct work_struct *work)
{
+ struct i915_mmu_object *mo = container_of(work, typeof(*mo), work);
+ struct drm_i915_gem_object *obj = mo->obj;
struct drm_device *dev = obj->base.dev;
- unsigned long end;
mutex_lock(&dev->struct_mutex);
/* Cancel any active worker and force us to re-evaluate gup */
@@ -89,46 +90,28 @@ static unsigned long cancel_userptr(struct
drm_i915_gem_object *obj)
dev_priv->mm.interruptible = was_interruptible;
}
- end = obj->userptr.ptr + obj->base.size;
-
drm_gem_object_unreference(&obj->base);
mutex_unlock(&dev->struct_mutex);
-
- return end;
}
-static void *invalidate_range__linear(struct i915_mmu_notifier *mn,
- struct mm_struct *mm,
- unsigned long start,
- unsigned long end)
+static unsigned long cancel_userptr(struct i915_mmu_object *mo)
{
- struct i915_mmu_object *mo;
- unsigned long serial;
-
-restart:
- serial = mn->serial;
- list_for_each_entry(mo, &mn->linear, link) {
- struct drm_i915_gem_object *obj;
-
- if (mo->it.last < start || mo->it.start > end)
- continue;
-
- obj = mo->obj;
-
- if (!mo->active ||
- !kref_get_unless_zero(&obj->base.refcount))
- continue;
-
- spin_unlock(&mn->lock);
-
- cancel_userptr(obj);
-
- spin_lock(&mn->lock);
- if (serial != mn->serial)
- goto restart;
+ unsigned long end = mo->obj->userptr.ptr + mo->obj->base.size;
+
+ /* The mmu_object is released late when destroying the
+* GEM object so it is entirely possible to gain a
+* reference on an object in the process of being freed
+* since our serialisation is via the spinlock and not
+* the struct_mutex - and consequently use it after it
+* is freed and then double free it.
+*/
+ if (mo->active && kref_get_unless_zero(&mo->obj->base.refcount)) {
+ schedule_work(&mo->work);
+ /* only schedule one work packet to avoid the refleak */
+ mo->active = false;
}
- return NULL;
+ return end;
}
static
Re: [Intel-gfx] [alsa-devel] DP MST audio support
Hi Dave, Sorry for interrupt. I'm currently testing the MST audio with HSW. Could you please help tell how I can say DP1.2 works? I connect a DP1.2 monitor to HSW and a DP1.1 monitor to DP1.2 monitor output. In this mode, can I make the 2 monitors show different contents? Thanks. Regards, Libin > -Original Message- > From: Dave Airlie [mailto:airl...@gmail.com] > Sent: Friday, June 19, 2015 6:38 PM > To: Lin, Mengdong > Cc: Takashi Iwai; Yang, Libin; Lu, Han; alsa-de...@alsa-project.org; intel- > g...@lists.freedesktop.org; Girdwood, Liam R > Subject: Re: [alsa-devel] DP MST audio support > > Just to fill in the info on this one. > > > We don't have Haswell Lenovo t440s atm, so could you share more > info? > > - Dell U2410 should support both HDMI and DP input. But I guess it > cannot > > support DP MST, right? > > - Are you connecting this monitor a DP cable? > > Which DDI port is used? DDI B, C or D? > > - Does audio fail after i915 enables DP MST? > > - Is patch "snd/hdmi: hack out haswell codec workaround" the only > change > > on audio driver side? > > Yes its a DP SST input on the U2410, and I'm using that for the audio. > > It's connected to the Lenovo dock with DP cable. The dock is an MST > device. > > The dock is connected to DDI C I think, and if the dock is operated in > SST > mode audio works, but in MST mode audio fails. (operating the dock in > SST > mode isn't useful though since only one of the multiple outputs works > then). > > >> > The graphics side patches are fairly trivial, also it would be good to > >> > get a good explaination of how the hw works, > >> > > >> > from what I can see devices get connections not pins on this hw, > and I > >> > notice that I don't always get 3 devices, so I'm not sure if devices > >> > are a dynamic thing we should be reprobing on some signal. > > > > Do you mean 3 PCM devices here, like pcmC0D3p, pcmC0D7p, > pcmC0D8p? > > Now the devices are not dynamic, a PCM device is created on each pin. > > It seems we need to revise this for DP MST, since a pin can be used to > send > > up to 3 independent streams on Intel GPU which has 3 display > pipelines. > > > > No I mean the "Devices" from snd_hda_get_devices. > > Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH mesa v3] i965/gen8+: bo in state base address must be in 32-bit address range
Hi,
Thanks for the comments,
On 8/7/2015 11:46 PM, Kristian Høgsberg wrote:
On Fri, Aug 7, 2015 at 2:45 AM, Michel Thierry wrote:
Gen8+ supports 48-bit virtual addresses, but some objects must always be
allocated inside the 32-bit address range.
In specific, any resource used with flat/heapless (0x-0xf000)
General State Heap or Intruction State Heap must be in a 32-bit range
(GSH / ISH), because the General State Offset and Instruction State Offset
are limited to 32-bits.
This doesn't look right. From the workaround text, it doesn't sound
like this is a HW problem, it only affects GMM. In mesa, we don't use
"heapless" (which I assume means setting the base to 0 and max range)
It is a hw problem, specifically in the state base address command.
General State Base Address and Instruction Base Address shouldn't be > 4GB.
for instructions, dynamic state or surface state. Surface state and
dynamic state is always in our batch bo which is limited to 8k.
Provided STATE_BASE_ADDRESS works correctly in the HW, the batch bo
can be places anywhere in the aperture. Offsets to dynamic or surface
state is relative to the beginning of the batch bo and will always be
less than 4g. Instructions are in their own bo (brw->cache.bo), but
again, we point instruction state base to it and all our shader stage
setup code references the instructions relative to the beginning of
the instruction bo.
I've seen the issue when the driver allocates mapped objects from bottom
to top and the other bo's from top to bottom (gpu hangs). So I say this
wa is needed.
-Michel
Kristian
Use drm_intel_bo_emit_reloc_48bit when the 4GB limit is not necessary, and
the bo can be in the full address space.
This commit introduces a dependency of libdrm 2.4.63, which introduces the
drm_intel_bo_emit_reloc_48bit function.
v2: s/48baddress/48b_address/,
Only use in OUT_RELOC64 cases, OUT_RELOC implies a 32-bit address offset
is needed (Ben)
v3: Added OUT_RELOC64_INSIDE_4G, so it stands out when a 64-bit relocation
needs the 32-bit workaround (Chris)
References:
http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html
Cc: Ben Widawsky
Cc: Chris Wilson
Signed-off-by: Michel Thierry
---
configure.ac | 2 +-
src/mesa/drivers/dri/i965/gen8_misc_state.c | 19 +++
src/mesa/drivers/dri/i965/intel_batchbuffer.c | 20
src/mesa/drivers/dri/i965/intel_batchbuffer.h | 10 --
4 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/configure.ac b/configure.ac
index af61aa2..c92ca44 100644
--- a/configure.ac
+++ b/configure.ac
@@ -68,7 +68,7 @@ AC_SUBST([OSMESA_VERSION])
dnl Versions for external dependencies
LIBDRM_REQUIRED=2.4.38
LIBDRM_RADEON_REQUIRED=2.4.56
-LIBDRM_INTEL_REQUIRED=2.4.60
+LIBDRM_INTEL_REQUIRED=2.4.63
LIBDRM_NVVIEUX_REQUIRED=2.4.33
LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
LIBDRM_FREEDRENO_REQUIRED=2.4.57
diff --git a/src/mesa/drivers/dri/i965/gen8_misc_state.c
b/src/mesa/drivers/dri/i965/gen8_misc_state.c
index b20038e..73eba06 100644
--- a/src/mesa/drivers/dri/i965/gen8_misc_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_misc_state.c
@@ -28,6 +28,10 @@
/**
* Define the base addresses which some state is referenced from.
+ *
+ * Use OUT_RELOC64_INSIDE_4G instead of OUT_RELOC64, the General State
+ * Offset and Instruction State Offset are limited to 32-bits by hardware,
+ * and must be located in the first 4GBs (32-bit offset).
*/
void gen8_upload_state_base_address(struct brw_context *brw)
{
@@ -41,19 +45,18 @@ void gen8_upload_state_base_address(struct brw_context *brw)
OUT_BATCH(0);
OUT_BATCH(mocs_wb << 16);
/* Surface state base address: */
- OUT_RELOC64(brw->batch.bo, I915_GEM_DOMAIN_SAMPLER, 0,
- mocs_wb << 4 | 1);
+ OUT_RELOC64_INSIDE_4G(brw->batch.bo, I915_GEM_DOMAIN_SAMPLER, 0,
+ mocs_wb << 4 | 1);
/* Dynamic state base address: */
- OUT_RELOC64(brw->batch.bo,
- I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION, 0,
- mocs_wb << 4 | 1);
+ OUT_RELOC64_INSIDE_4G(brw->batch.bo,
+ I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
0,
+ mocs_wb << 4 | 1);
/* Indirect object base address: MEDIA_OBJECT data */
OUT_BATCH(mocs_wb << 4 | 1);
OUT_BATCH(0);
/* Instruction base address: shader kernels (incl. SIP) */
- OUT_RELOC64(brw->cache.bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
- mocs_wb << 4 | 1);
-
+ OUT_RELOC64_INSIDE_4G(brw->cache.bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
+ mocs_wb << 4 | 1);
/* General state buffer size */
OUT_BATCH(0xf001);
/* Dynamic state buffer size */
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 54081a1..ca90784 100644
--- a/src/mesa/drivers/dri/i965/
Re: [Intel-gfx] [PATCH] drm/i915:gen9: Add WA for disable gather at set shader bit
On 08/08/2015 06:35, Ben Widawsky wrote:
On Fri, Aug 07, 2015 at 06:33:37PM +0100, Arun Siluvery wrote:
This WA doesn't have a name. According to the spec, driver need to reset
disable gather at set shader bit in per ctx WA batch. It is to be noted
that the default value is already '0' for this bit but we still need to
apply this WA because userspace may set it. If userspace really need it
to be set then they need to do in every batch.
Cc: Ben Widawsky
Cc: Mika Kuoppala
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_lrc.c | 9 +
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ea46d68..838537f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5834,6 +5834,7 @@ enum skl_disp_power_wells {
# define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC((1<<10) | (1<<26))
# define GEN9_RHWO_OPTIMIZATION_DISABLE (1<<14)
#define COMMON_SLICE_CHICKEN2 0x7014
+#define GEN9_DISABLE_GATHER_SET_SHADER_SLICE (1<<12)
# define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE (1<<0)
#define HIZ_CHICKEN 0x7018
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4c40614..df3bb98 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct intel_engine_cs
*ring,
struct drm_device *dev = ring->dev;
uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
+ /* WaNoName:skl,bxt
+* This WA has no name, according to the spec driver needs to reset
+* "disable gather at set shader slice" bit in per ctx batch
+*/
+ wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
+ wa_ctx_emit(batch, index, COMMON_SLICE_CHICKEN2);
+ wa_ctx_emit(batch, index,
+ _MASKED_BIT_DISABLE(GEN9_DISABLE_GATHER_SET_SHADER_SLICE));
+
/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
(IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
Hmm. I thought we needed this, but looking at the "User Mode Privileged
Commands" of the spec, it seems like this register is not allowed to be written.
So unless this register is put in a whitelist somewhere in the future, I think
it's safe to drop this patch.
We need to whitelist few registers for preemption related WA, this can
be added to whitelist if userspace really needs to write to it.
regards
Arun
As a preventative measure, I don't see this as harmful - but I don't feel I have
any authority to suggest whether we keep this in or not.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/doc: Update docs about device instance setup
->load is depracated, bus functionst are deprecated and everyone should use drm_dev_alloc®ister. So update the .tmpl (and pull a bunch of the overview docs into the sourcecode to increase chances that it'll stay in sync in the future) and add notes to functions which are deprecated. I didn't bother to clean up and document the unload sequence similarly since that one is still a bit a mess: drm_dev_unregister does way too much, drm_unplug_dev does what _unregister should be doing but then has the complication of promising something it doesn't actually do (it doesn't unplug existing open fds for instance, only prevents new ones). Motivated since I don't want to hunt every new driver for usage of drm_platform_init any more ;-) Signed-off-by: Daniel Vetter --- Documentation/DocBook/drm.tmpl | 99 -- drivers/gpu/drm/drm_drv.c | 55 +-- drivers/gpu/drm/drm_pci.c | 11 + drivers/gpu/drm/drm_platform.c | 3 ++ 4 files changed, 83 insertions(+), 85 deletions(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ec34b9becebd..f1884038b90f 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -138,14 +138,10 @@ At the core of every DRM driver is a drm_driver structure. Drivers typically statically initialize a drm_driver structure, - and then pass it to one of the drm_*_init() functions - to register it with the DRM subsystem. - - - Newer drivers that no longer require a drm_bus - structure can alternatively use the low-level device initialization and - registration functions such as drm_dev_alloc() and - drm_dev_register() directly. + and then pass it to drm_dev_alloc() to allocate a + device instance. After the device instance is fully initialized it can be + registered (which makes it accessible from userspace) using + drm_dev_register(). The drm_driver structure contains static @@ -296,83 +292,12 @@ char *date; - Device Registration - -A number of functions are provided to help with device registration. -The functions deal with PCI and platform devices, respectively. - -!Edrivers/gpu/drm/drm_pci.c -!Edrivers/gpu/drm/drm_platform.c - -New drivers that no longer rely on the services provided by the -drm_bus structure can call the low-level -device registration functions directly. The -drm_dev_alloc() function can be used to allocate -and initialize a new drm_device structure. -Drivers will typically want to perform some additional setup on this -structure, such as allocating driver-specific data and storing a -pointer to it in the DRM device's dev_private -field. Drivers should also set the device's unique name using the -drm_dev_set_unique() function. After it has been -set up a device can be registered with the DRM subsystem by calling -drm_dev_register(). This will cause the device to -be exposed to userspace and will call the driver's -.load() implementation. When a device is -removed, the DRM device can safely be unregistered and freed by calling -drm_dev_unregister() followed by a call to -drm_dev_unref(). - + Device Instance and Driver Handling +!Pdrivers/gpu/drm/drm_drv.c driver instance overview !Edrivers/gpu/drm/drm_drv.c Driver Load - -The load method is the driver and device -initialization entry point. The method is responsible for allocating and - initializing driver private data, performing resource allocation and - mapping (e.g. acquiring -clocks, mapping registers or allocating command buffers), initializing -the memory manager (), installing -the IRQ handler (), setting up -vertical blanking handling (), mode - setting () and initial output - configuration (). - - -If compatibility is a concern (e.g. with drivers converted over from -User Mode Setting to Kernel Mode Setting), care must be taken to prevent -device initialization and control that is incompatible with currently -active userspace drivers. For instance, if user level mode setting -drivers are in use, it would be problematic to perform output discovery -& configuration at load time. Likewise, if user-level drivers -unaware of memory management are in use, memory management and command -buffer setup may need to be omitted. These requirements are -driver-specific, and care needs to be taken to keep both old and new -applications and libraries working. - - int (*load) (struct drm_device *, unsigned long flags); - -The method takes two arguments, a pointer to the newly created -
[Intel-gfx] [PATCH 1/2] drm/edid: Use ARRAY_SIZE in drm_add_modes_noedid
Spotted while reading code for random reasons. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_edid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 4a403eb90ded..4780b1924bef 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3810,7 +3810,7 @@ int drm_add_modes_noedid(struct drm_connector *connector, struct drm_display_mode *mode; struct drm_device *dev = connector->dev; - count = sizeof(drm_dmt_modes) / sizeof(struct drm_display_mode); + count = ARRAY_SIZE(drm_dmt_modes); if (hdisplay < 0) hdisplay = 0; if (vdisplay < 0) -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: disable_shared_pll doesn't work on pre-gen5
On Mon, Aug 03, 2015 at 01:09:11PM -0700, Jesse Barnes wrote: > Looks like > > commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6 > Author: Maarten Lankhorst > Date: Mon Jun 15 12:33:53 2015 +0200 > drm/i915: Update less state during modeset. > > introduced the unconditional calling of disable_shared_dpll, but didn't > fix up pre-gen5 to avoid the BUG_ON at the top of the function. > > So change the BUG_ON into a gen check (alternately we could move the > BUG_ON until later, since we shouldn't have a pll struct here either, > but this seems clearer to read). > > This fixes a crash on load on my x200s platform. > > Signed-off-by: Jesse Barnes This blows up in 4.1 (a BUG_ON during boot causing a hard lockup). -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 1/2] drm/tegra: don't take dev->struct_mutex in mmap offset ioctl
On Wed, Jul 15, 2015 at 03:38:51PM +0200, Daniel Vetter wrote:
> Since David Herrmann's mmap vma manager rework we don't need to grab
> dev->struct_mutex any more to prevent races when looking up the mmap
> offset. Drop it and instead don't forget to use the unref_unlocked
> variant (since the drm core still cares).
>
> While at it also fix a leak when this ioctl is called on an imported
> buffer.
I don't see where the leak's fixed, but other than that this looks good
to me. Shall I pick this up into the drm/tegra tree?
Thierry
> Cc: Thierry Reding
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/tegra/gem.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> index 01e16e146bfe..827838e64d6e 100644
> --- a/drivers/gpu/drm/tegra/gem.c
> +++ b/drivers/gpu/drm/tegra/gem.c
> @@ -408,12 +408,9 @@ int tegra_bo_dumb_map_offset(struct drm_file *file,
> struct drm_device *drm,
> struct drm_gem_object *gem;
> struct tegra_bo *bo;
>
> - mutex_lock(&drm->struct_mutex);
> -
> gem = drm_gem_object_lookup(drm, file, handle);
> if (!gem) {
> dev_err(drm->dev, "failed to lookup GEM object\n");
> - mutex_unlock(&drm->struct_mutex);
> return -EINVAL;
> }
>
> @@ -423,8 +420,6 @@ int tegra_bo_dumb_map_offset(struct drm_file *file,
> struct drm_device *drm,
>
> drm_gem_object_unreference(gem);
>
> - mutex_unlock(&drm->struct_mutex);
> -
> return 0;
> }
>
> --
> 2.1.4
>
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Retry port as HDMI if dp_is_edp turns out to be false
Chris,
After asking few people found that the simplest explanation and
plausible root cause
being that PORT D is set for both eDP and HDMI :). in which case the
proper fix is to
check in intel_dp_is_edp, if PORT D is again set for any other display
and return false
if that is the case
or
have a quirk for this config since this seems to be reported atleast for
now as suggested
by Lukas
i would recommend the first method, if we confirm the root cause is as
explained above.
On 8/10/2015 11:35 AM, Sivakumar Thulasimani wrote:
hi Mengdong,
is there any reason why you cannot modify VBT ? unless it is
shipped version you
can just flash the modified VBT along with BIOS.
Chris,
i would be even more surprised if VBIOS/GOP can enable some
display when it is
configured incorrectly in VBT. Give me a day to check with someone if
this is even
possible in that level.
On 8/10/2015 8:00 AM, Lin, Mengdong wrote:
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
Behalf Of
Lukas Wunner
Hi,
On Sun, Aug 09, 2015 at 01:12:53PM +0100, Chris Wilson wrote:
We follow the VBT as to whether a DDI port is used for eDP and if so,
do not attach a HDMI encoder to it. However there are machines for
which the VBT eDP flag is a lie (shocking!) and we fail to detect a
eDP link.
Furthermore, on those machines the HDMI is connected to that DDI port
but we ignore it.
If we reorder our port initialisation to try the eDP setup first and
if that fails we can treat it as a normal DP port along with a HDMI
output. To accomplish this, we have to delay registering the DP
connector/encoder until after we establish its final type.
Sorry to jump in. Could this help another use case as below ?
We have some Bytrail machine (Bayley Bay), we applied HW rework to
disable eDP connectivity to DDI1 and enable DP on DDI 1.
But we found the i915 driver still take this DDI as eDP, not DP. we
suspect it's because VBT still takes it as DP and so i915 driver just
follows.
If we don't want to revise VBT in BIOS after every rework, is there
any other way to let i915 detect this is a DP link?
Thanks
Mengdong
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88331
The existing code seems very carefully crafted, taking into account the
differences betweem various GPU generations etc, shuffling that around
might risk breakage. FWIW, I'm wondering if just adding a quirk for
this single
Dell workstation might be justified?
Best regards,
Lukas
Signed-off-by: Chris Wilson
Cc: Jesse Barnes
---
drivers/gpu/drm/i915/intel_display.c | 27
drivers/gpu/drm/i915/intel_dp.c | 127
+--
drivers/gpu/drm/i915/intel_drv.h | 6 +-
3 files changed, 79 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0bcd1b1aba4f..aab8dfd1f8a5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13941,7 +13941,6 @@ static void intel_setup_outputs(struct
drm_device *dev) {
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_encoder *encoder;
-bool dpd_is_edp = false;
intel_lvds_init(dev);
@@ -13983,31 +13982,33 @@ static void intel_setup_outputs(struct
drm_device *dev)
intel_ddi_init(dev, PORT_D);
} else if (HAS_PCH_SPLIT(dev)) {
int found;
-dpd_is_edp = intel_dp_is_edp(dev, PORT_D);
if (has_edp_a(dev))
intel_dp_init(dev, DP_A, PORT_A);
+found = 0;
+/* PCH SDVOB multiplex with HDMIB */
if (I915_READ(PCH_HDMIB) & SDVO_DETECTED) {
-/* PCH SDVOB multiplex with HDMIB */
found = intel_sdvo_init(dev, PCH_SDVOB, true);
if (!found)
intel_hdmi_init(dev, PCH_HDMIB, PORT_B);
-if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED))
-intel_dp_init(dev, PCH_DP_B, PORT_B);
}
+if (!found && I915_READ(PCH_DP_B) & DP_DETECTED)
+intel_dp_init(dev, PCH_DP_B, PORT_B);
-if (I915_READ(PCH_HDMIC) & SDVO_DETECTED)
-intel_hdmi_init(dev, PCH_HDMIC, PORT_C);
-
-if (!dpd_is_edp && I915_READ(PCH_HDMID) & SDVO_DETECTED)
-intel_hdmi_init(dev, PCH_HDMID, PORT_D);
-
+found = 0;
if (I915_READ(PCH_DP_C) & DP_DETECTED)
-intel_dp_init(dev, PCH_DP_C, PORT_C);
+found = intel_dp_init(dev, PCH_DP_C, PORT_C);
+if (found != DRM_MODE_CONNECTOR_eDP &&
+I915_READ(PCH_HDMIC) & SDVO_DETECTED)
+intel_hdmi_init(dev, PCH_HDMIC, PORT_C);
+found = 0;
if (I915_READ(PCH_DP_D) & DP_DETECTED)
-intel_dp_init(dev, PCH_DP_D, PORT_D);
+found = intel_dp_init(dev, PCH_DP_D, PORT_D);
+if (found != DRM_MODE_CONNECTOR_eDP &&
+I915_READ(PCH_HDMID) & SDVO_DETECTED)
+intel
Re: [Intel-gfx] [PATCH 02/18] drm/cma-helper: Fix locking in drm_fb_cma_debugfs_show
On Thu, Jul 09, 2015 at 11:32:34PM +0200, Daniel Vetter wrote: > This function takes two locks, both of them the wrong ones. This > wasn't an oversight from my fb locking rework since both patches > landed in parallel. We really only need fb_lock when walking that > list, since everything we can reach from that is refcounted properly > already. > > Cc: Rob Clark > Cc: Laurent Pinchart > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_fb_cma_helper.c | 16 ++-- > drivers/gpu/drm/drm_gem_cma_helper.c | 2 -- > 2 files changed, 2 insertions(+), 16 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/18] drm/gem: Be more friendly with locking checks
On Thu, Jul 09, 2015 at 11:32:35PM +0200, Daniel Vetter wrote: > BUG_ON kills the driver, WARN_ON is much friendly. And usually nothing > bad happens when the locking is lightly busted. s/much friendly/much friendlier/, s/lightly/slightly/? Otherwise: Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Retry port as HDMI if dp_is_edp turns out to be false
On Mon, Aug 10, 2015 at 04:05:31PM +0530, Sivakumar Thulasimani wrote: > Chris, > After asking few people found that the simplest explanation and > plausible root cause > being that PORT D is set for both eDP and HDMI :). in which case the > proper fix is to > check in intel_dp_is_edp, if PORT D is again set for any other > display and return false > if that is the case The only difference between that and my patch is that you would not expose the DP link if the VBT flagged it as eDP. -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 05/18] drm/bochs: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:37PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/bochs/bochs_mm.c | 16 > 1 file changed, 4 insertions(+), 12 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/18] drm/ast: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:36PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/ast/ast_main.c | 16 +--- > 1 file changed, 5 insertions(+), 11 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/18] drm/mga200g: Hold a proper reference for cursor_set
On Thu, Jul 09, 2015 at 11:32:39PM +0200, Daniel Vetter wrote: > Looking up an obj, immediate dropping the acquired reference and then > continuing to use it isn't how this is supposed to work. Fix this by > holding a reference for the entire function. > > While at it stop grabbing dev->struct_mutex, it doesn't protect > anything here. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/mgag200/mgag200_cursor.c | 22 ++ > 1 file changed, 10 insertions(+), 12 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/18] drm/mga200g: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:38PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/mgag200/mgag200_main.c | 16 > 1 file changed, 4 insertions(+), 12 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 17/18] drm/amdgpu: Don't take dev->struct_mutex in bo_force_delete
On Thu, Jul 09, 2015 at 11:32:49PM +0200, Daniel Vetter wrote: > It really doesn't protect anything which doesn't have other locks > already. Also this is run from driver unload code so not much need for > locks anyway. > > Same changes as for readone really. s/radeone/radeon/ Otherwise: Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/18] drm/cirrus: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:40PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/cirrus/cirrus_main.c | 15 --- > 1 file changed, 4 insertions(+), 11 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/18] drm/cma-helper: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:41PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Cc: Laurent Pinchart > Cc: Rob Clark > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_gem_cma_helper.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/18] drm/rockchip: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:42PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Aside: I stumbled over the mmap handler which directly does a > dma_mmap_attrs. But totally fails to grab a reference on the > underlying object and hence looks like it happily just leaks the ptes > since there's no guarantee the mmap isn't still around when > gem_free_object is called. Which the kerneldoc of dma_mmap_attrs > explicitly forbids. Same is true for Exynos, which seems to be the source for copy/paste here. Anyway, for this change: Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Spam less on dp aux send/receive problems
On Mon, 10 Aug 2015, Jani Nikula wrote:
> On Thu, 06 Aug 2015, Mika Kuoppala wrote:
>> If we encounter frequent problems with dp aux channel
>> communications, we end up spamming the dmesg with the
>> exact similar trace and status.
>>
>> Inject a new backtrace only if we have new information
>> to share as otherwise we flush out all other important
>> stuff.
>>
>> Signed-off-by: Mika Kuoppala
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 11 +--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index df7e2cf..6b2f7af 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -849,8 +849,15 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>> }
>>
>> if (try == 3) {
>> -WARN(1, "dp_aux_ch not started status 0x%08x\n",
>> - I915_READ(ch_ctl));
>> +static u32 last_status = -1;
>> +const u32 status = I915_READ(ch_ctl);
>> +
>> +if (status != last_status) {
>> +WARN(1, "dp_aux_ch not started status 0x%08x\n",
>> + status);
>> +last_status = status;
>> +}
>> +
>
> But now you'll also skip the logging even if there's been a day and a
> million successful transfers since the last error... I understand your
> concern, but if you feel you must do something like this, please at
> least reset last_status on success.
Hmh, I see now that this has already been merged... :(
>
> BR,
> Jani.
>
>
>> ret = -EBUSY;
>> goto out;
>> }
>> --
>> 2.1.4
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/18] drm/armada: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:43PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > While at it also fix a leak when this ioctl is called on an imported > buffer. > > Cc: Russell King > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/armada/armada_gem.c | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/18] drm/nouveau: Don't take dev->struct_mutex in fbcon init
On Thu, Jul 09, 2015 at 11:32:44PM +0200, Daniel Vetter wrote: > It doesn't protect anything at all. fbdev helper state is all > protected by modeset locks, and nouveau bo state is taken care of by > ttm. There's also nothing else grabbing struct_mutex that might need > to coordinate with this code. Also this is driver load code, no one > can get at the device yet anyway so locking is fairly futile. > There's also no drm_gem_object_unreference that would now suddenly > need the _unlocked variant. > > Cc: Ben Skeggs > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/nouveau/nouveau_fbcon.c | 11 +++ > 1 file changed, 3 insertions(+), 8 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/18] drm/nouveau: Don't take dev->struct_mutex in ttm_fini
On Thu, Jul 09, 2015 at 11:32:45PM +0200, Daniel Vetter wrote: > This is only called in driver load/unload paths, no need to grab any > locks at all. Also, ttm takes care of itself anyway. > > Cc: Ben Skeggs > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 -- > 1 file changed, 2 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/18] drm/qxl: Don't take dev->struct_mutex in bo_force_delete
On Thu, Jul 09, 2015 at 11:32:46PM +0200, Daniel Vetter wrote: > It really doesn't protect anything which doesn't have other locks > already. It also doesn't seem to be wired up into the driver unload > code fwiw, but that's a different issue. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/qxl/qxl_object.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 15/18] drm/radeon: Don't take dev->struct_mutex in bo_force_delete
On Thu, Jul 09, 2015 at 11:32:47PM +0200, Daniel Vetter wrote: > It really doesn't protect anything which doesn't have other locks > already. Also this is run from driver unload code so not much need for > locks anyway. > > Cc: Alex Deucher > Cc: "Christian König" > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/radeon/radeon_object.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 16/18] drm/radeon: Don't take dev->struct_mutex in pm functions
On Thu, Jul 09, 2015 at 11:32:48PM +0200, Daniel Vetter wrote: > We already grab 2 device-global locks (write-sema rdev->pm.mclk_lock > and rdev->ring_lock), adding another global mutex won't serialize this > code more. And since there's really nothing interesting that gets > protected in radeon by dev->struct mutex (we only have the global z > buffer owners and it's still serializing gem bo destruction in the drm > core - which is irrelevant since radeon uses ttm anyway internally) > this doesn't add protection. Remove it. > > Cc: Alex Deucher > Cc: "Christian König" > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/radeon/radeon_pm.c | 5 - > 1 file changed, 5 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/18] drm/amdgpu: don't grab dev->struct_mutex in pm functions
On Thu, Jul 09, 2015 at 11:32:50PM +0200, Daniel Vetter wrote: > Similar to radeon, except that amdgpu doesn't even use struct_mutex to > protect anything like the shared z buffer (sane gpu architecture, > yay!). And the code already grabs the globa adev->ring_lock, so this > code can't race with itself. Which makes struct_mutex completely > redundnant. Remove it. > > Cc: Alex Deucher > Cc: "Christian König" > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 2 -- > 1 file changed, 2 deletions(-) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/18] drm/armada: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:43PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > While at it also fix a leak when this ioctl is called on an imported > buffer. Good catch, thanks Daniel. I'd prefer these to be two different commits though, so that the leak can be easily backported to stable kernels if needed. I suspect this should go through the same tree that David's work is, otherwise we end up with random dependencies between trees. With the bug fix separated out: Acked-by: Russell King -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/tegra: don't take dev->struct_mutex in mmap offset ioctl
On Mon, Aug 10, 2015 at 12:30:21PM +0200, Thierry Reding wrote:
> On Wed, Jul 15, 2015 at 03:38:51PM +0200, Daniel Vetter wrote:
> > Since David Herrmann's mmap vma manager rework we don't need to grab
> > dev->struct_mutex any more to prevent races when looking up the mmap
> > offset. Drop it and instead don't forget to use the unref_unlocked
> > variant (since the drm core still cares).
> >
> > While at it also fix a leak when this ioctl is called on an imported
> > buffer.
>
> I don't see where the leak's fixed, but other than that this looks good
> to me. Shall I pick this up into the drm/tegra tree?
Copypaste in the commit message from armada, doesn't apply to tegra. Do
you also plan to pick up "drm/tegra: Use
drm_gem_object_reference_unlocked" directly?
And thanks for all the review.
-Daniel
>
> Thierry
>
> > Cc: Thierry Reding
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/tegra/gem.c | 5 -
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> > index 01e16e146bfe..827838e64d6e 100644
> > --- a/drivers/gpu/drm/tegra/gem.c
> > +++ b/drivers/gpu/drm/tegra/gem.c
> > @@ -408,12 +408,9 @@ int tegra_bo_dumb_map_offset(struct drm_file *file,
> > struct drm_device *drm,
> > struct drm_gem_object *gem;
> > struct tegra_bo *bo;
> >
> > - mutex_lock(&drm->struct_mutex);
> > -
> > gem = drm_gem_object_lookup(drm, file, handle);
> > if (!gem) {
> > dev_err(drm->dev, "failed to lookup GEM object\n");
> > - mutex_unlock(&drm->struct_mutex);
> > return -EINVAL;
> > }
> >
> > @@ -423,8 +420,6 @@ int tegra_bo_dumb_map_offset(struct drm_file *file,
> > struct drm_device *drm,
> >
> > drm_gem_object_unreference(gem);
> >
> > - mutex_unlock(&drm->struct_mutex);
> > -
> > return 0;
> > }
> >
> > --
> > 2.1.4
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
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 01/18] drm/gem: rip out drm vma accounting for gem mmaps
On Thu, Jul 09, 2015 at 11:32:33PM +0200, Daniel Vetter wrote: > Doesn't really add anything which can't be figured out through > proc files. And more clearly separates the new gem mmap handling > code from the old drm maps mmap handling code, which is surely a > good thing. > > Cc: Martin Peres > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_gem.c | 11 +-- > 1 file changed, 1 insertion(+), 10 deletions(-) Doesn't this mean that the "vma" debugfs file will now be empty for GEM drivers? Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Use CONFIG_DRM_FBDEV_EMULATION
Instead of our own duplicated one. This fixes a bug in the driver
unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we
try to unregister the nonexistent fbdev drm_framebuffer.
Cc: Archit Taneja
Cc: Maarten Lankhorst
Reported-by: Maarten Lankhorst
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/Kconfig | 15 ---
drivers/gpu/drm/i915/Makefile| 2 +-
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_display.c | 4 ++--
drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++--
drivers/gpu/drm/i915/intel_drv.h | 2 +-
7 files changed, 8 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index eb87e2538861..051eab33e4c7 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -36,21 +36,6 @@ config DRM_I915
i810 driver instead, and the Atom z5xx series has an entirely
different implementation.
-config DRM_I915_FBDEV
- bool "Enable legacy fbdev support for the modesetting intel driver"
- depends on DRM_I915
- select DRM_KMS_FB_HELPER
- select FB_CFB_FILLRECT
- select FB_CFB_COPYAREA
- select FB_CFB_IMAGEBLIT
- default y
- help
- Choose this option if you have a need for the legacy fbdev
- support. Note that this support also provide the linux console
- support on top of the intel modesetting driver.
-
- If in doubt, say "Y".
-
config DRM_I915_PRELIMINARY_HW_SUPPORT
bool "Enable preliminary support for prerelease Intel hardware by
default"
depends on DRM_I915
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 41fb8a9c5bef..998b4643109f 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -62,7 +62,7 @@ i915-y += intel_audio.o \
intel_sideband.o \
intel_sprite.o
i915-$(CONFIG_ACPI)+= intel_acpi.o intel_opregion.o
-i915-$(CONFIG_DRM_I915_FBDEV) += intel_fbdev.o
+i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o
# modesetting output/encoder code
i915-y += dvo_ch7017.o \
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 95e7b82f05d1..86734be84a65 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1868,7 +1868,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m,
void *data)
struct intel_framebuffer *fb;
struct drm_framebuffer *drm_fb;
-#ifdef CONFIG_DRM_I915_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
struct drm_i915_private *dev_priv = dev->dev_private;
ifbdev = dev_priv->fbdev;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4932d298e3af..95c115f6a4c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1855,7 +1855,7 @@ struct drm_i915_private {
struct drm_i915_gem_object *vlv_pctx;
-#ifdef CONFIG_DRM_I915_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
/* list of fbdev register on this device */
struct intel_fbdev *fbdev;
struct work_struct fbdev_suspend_work;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 16c8052f7eab..9a2f229a1c3a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10131,7 +10131,7 @@ static struct drm_framebuffer *
mode_fits_in_fbdev(struct drm_device *dev,
struct drm_display_mode *mode)
{
-#ifdef CONFIG_DRM_I915_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj;
struct drm_framebuffer *fb;
@@ -14313,7 +14313,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
return intel_framebuffer_create(dev, mode_cmd, obj);
}
-#ifndef CONFIG_DRM_I915_FBDEV
+#ifndef CONFIG_DRM_FBDEV_EMULATION
static inline void intel_fbdev_output_poll_changed(struct drm_device *dev)
{
}
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index f4fe1183bae6..369f8b6b804f 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -406,7 +406,7 @@ static bool intel_dp_mst_get_hw_state(struct
intel_connector *connector)
static void intel_connector_add_to_fbdev(struct intel_connector *connector)
{
-#ifdef CONFIG_DRM_I915_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper,
&connector->base);
#endif
@@ -414,7 +414,7 @@ static void intel_connector_add_to_fbdev(struct
intel_connector *connector)
static void intel_connector_remove_from_fbdev(struct intel_connector
*connector)
{
-#ifdef CONFIG_DRM_I915_FBDEV
+#ifdef CONFIG_DRM_FBDEV_EMULATION
str
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Dont enable hpd for eDP
On Mon, 10 Aug 2015, Sivakumar Thulasimani
wrote:
> Reviewed-by: Sivakumar Thulasimani
>
> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
>> With HPD support added for all ports including PORT_A, setting hpd_pin will
>> result in enabling of hpd to edp as well. There is no need to enable HPD on
>> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
>> be connected. it can be added back when required
What? You can't just go ahead and remove HPD from eDP sinks.
BR,
Jani.
>>
>> Signed-off-by: Sonika Jindal
>> ---
>> drivers/gpu/drm/i915/intel_dp.c |2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index fcc64e5..5a614c9 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port
>> *intel_dig_port,
>> /* Set up the hotplug pin. */
>> switch (port) {
>> case PORT_A:
>> -intel_encoder->hpd_pin = HPD_PORT_A;
>> +/* Not enabling edp interrupts */
>> break;
>> case PORT_B:
>> intel_encoder->hpd_pin = HPD_PORT_B;
>
> --
> regards,
> Sivakumar
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/18] dev->struct_mutex crusade
On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote: > Hi all, > > I wanted to take another look at struct_mutex usage in modern (gem) drivers > and > noticed that for a fair lot we're very to be completely struct_mutex free. > > This pile here is the the simple part, which mostly just removes code and > mutex_lock/unlock calls. All the patches here are independent and can be > merged > in any order whatsoever. My plan is to send out a pull request for all those > not > picked up by driver maintainers in 2-3 weeks or so, assuming no one complains. > > Of course review & comments still very much welcome. > > The more tricky 2nd part of this (and that one's not yet done) is to rework > the > gem mmap handler to use the same kref_get_unless_zero trick as ttm. With that > there's no core requirement to hold struct_mutex over the final unref, which > means we can make that one lockless. I plan to add a gem_object_free_unlocked > for all the drivers which don't have any need for this lock. > > Also there's a few more drivers which can be made struct_mutex free easily, > I'll > propably stitch together poc patches for those. There's a concurrency bug in Tegra DRM currently because we don't lock accesses to drm_mm (I guess this demonstrates how badly we need better testing...) and it seems like this is typically protected by the very same struct_mutex that you're on a crusade to remove. If your goal is to get rid of it for good, should we simply add a separate lock just for the drm_mm? We don't have another one that would fit. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/tegra: don't take dev->struct_mutex in mmap offset ioctl
On Mon, Aug 10, 2015 at 01:31:42PM +0200, Daniel Vetter wrote: > On Mon, Aug 10, 2015 at 12:30:21PM +0200, Thierry Reding wrote: > > On Wed, Jul 15, 2015 at 03:38:51PM +0200, Daniel Vetter wrote: > > > Since David Herrmann's mmap vma manager rework we don't need to grab > > > dev->struct_mutex any more to prevent races when looking up the mmap > > > offset. Drop it and instead don't forget to use the unref_unlocked > > > variant (since the drm core still cares). > > > > > > While at it also fix a leak when this ioctl is called on an imported > > > buffer. > > > > I don't see where the leak's fixed, but other than that this looks good > > to me. Shall I pick this up into the drm/tegra tree? > > Copypaste in the commit message from armada, doesn't apply to tegra. Do > you also plan to pick up "drm/tegra: Use > drm_gem_object_reference_unlocked" directly? I don't mind much either way. I don't think they'll conflict with anything, but since you already said that they're all independent, I don't see a reason why I shouldn't pull them into drm/tegra. If you prefer to keep them together, that's fine with me too. Thanks, Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Dont enable hpd for eDP
On 8/10/2015 5:07 PM, Jani Nikula wrote:
On Mon, 10 Aug 2015, Sivakumar Thulasimani
wrote:
Reviewed-by: Sivakumar Thulasimani
On 8/10/2015 10:35 AM, Sonika Jindal wrote:
With HPD support added for all ports including PORT_A, setting hpd_pin will
result in enabling of hpd to edp as well. There is no need to enable HPD on
PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
be connected. it can be added back when required
What? You can't just go ahead and remove HPD from eDP sinks.
BR,
Jani.
Nope, we are not removing HPD for edp sinks, it was never there in the
first place. It was
enabled for CHV (even there by mistake since PORT B/C was both DP and
eDP) but it was
never there for any other plaforms nor is it used for any purpose (PSR
must use it, but i
dont see code for it as well).
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/intel_dp.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fcc64e5..5a614c9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
/* Set up the hotplug pin. */
switch (port) {
case PORT_A:
- intel_encoder->hpd_pin = HPD_PORT_A;
+ /* Not enabling edp interrupts */
break;
case PORT_B:
intel_encoder->hpd_pin = HPD_PORT_B;
--
regards,
Sivakumar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
regards,
Sivakumar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/18] drm/gem: Be more friendly with locking checks
On Mon, Aug 10, 2015 at 12:42:30PM +0200, Thierry Reding wrote: > On Thu, Jul 09, 2015 at 11:32:35PM +0200, Daniel Vetter wrote: > > BUG_ON kills the driver, WARN_ON is much friendly. And usually nothing > > bad happens when the locking is lightly busted. > > s/much friendly/much friendlier/, s/lightly/slightly/? Yeah, changed. > Otherwise: > > Reviewed-by: Thierry Reding Thanks for your review, I've applied the patches to topic/drm-misc except armada (needs to be split), one nouveau patch (superseeded meanwhile by Archit's work), radeon/amdgpu (I'll ping Alex) and the two tegra ones. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 v2 1/4] drm/i915: Add audio set_ncts callback
On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> From: Libin Yang
>
> Add the set_ncts callback.
>
> With the callback, audio driver can trigger
> i915 driver to set the proper N/CTS
> based on different sample rates.
>
> Signed-off-by: Libin Yang
> ---
> include/drm/i915_component.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index c9a8b64..7305881 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -33,6 +33,8 @@ struct i915_audio_component {
> void (*put_power)(struct device *);
> void (*codec_wake_override)(struct device *, bool enable);
> int (*get_cdclk_freq)(struct device *);
> + int (*set_ncts)(struct device *, int port, int dev_entry,
> + int rate);
I'd rather call this set_audio_rate or similar, and dropping the
references to N and CTS. The caller does not need to know.
I'm also not fond of adding a dev_entry parameter and leaving it
unused. I do not think we know specifically how we're going to identify
MST sinks, and the interface may need to be changed anyway. Let's force
an update in the caller side as well when there's actually sensible
support in our side.
BR,
Jani.
> } *ops;
> };
>
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use CONFIG_DRM_FBDEV_EMULATION
On Mon, Aug 10, 2015 at 01:34:08PM +0200, Daniel Vetter wrote: > Instead of our own duplicated one. This fixes a bug in the driver > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > try to unregister the nonexistent fbdev drm_framebuffer. > > Cc: Archit Taneja > Cc: Maarten Lankhorst > Reported-by: Maarten Lankhorst > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/Kconfig | 15 --- > drivers/gpu/drm/i915/Makefile| 2 +- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > 7 files changed, 8 insertions(+), 23 deletions(-) Isn't this going to cause some pain to users because .config may not have this symbol yet? Arguably this is somewhat mitigated by the fact that both symbols are "default y", but technically somebody could have DRM_I915_FBDEV=n in their .config and after this change fbdev emulation will be switched on again. I'm not sure how to upgrade more sanely, though, so perhaps this is just a bullet that needs biting. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/18] dev->struct_mutex crusade
On Mon, Aug 10, 2015 at 01:35:58PM +0200, Thierry Reding wrote: > On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote: > > Hi all, > > > > I wanted to take another look at struct_mutex usage in modern (gem) drivers > > and > > noticed that for a fair lot we're very to be completely struct_mutex free. > > > > This pile here is the the simple part, which mostly just removes code and > > mutex_lock/unlock calls. All the patches here are independent and can be > > merged > > in any order whatsoever. My plan is to send out a pull request for all > > those not > > picked up by driver maintainers in 2-3 weeks or so, assuming no one > > complains. > > > > Of course review & comments still very much welcome. > > > > The more tricky 2nd part of this (and that one's not yet done) is to rework > > the > > gem mmap handler to use the same kref_get_unless_zero trick as ttm. With > > that > > there's no core requirement to hold struct_mutex over the final unref, which > > means we can make that one lockless. I plan to add a > > gem_object_free_unlocked > > for all the drivers which don't have any need for this lock. > > > > Also there's a few more drivers which can be made struct_mutex free easily, > > I'll > > propably stitch together poc patches for those. > > There's a concurrency bug in Tegra DRM currently because we don't lock > accesses to drm_mm (I guess this demonstrates how badly we need better > testing...) and it seems like this is typically protected by the very > same struct_mutex that you're on a crusade to remove. If your goal is > to get rid of it for good, should we simply add a separate lock just > for the drm_mm? We don't have another one that would fit. Actually that is one of the first targets for more fine-grained locking. I would not add a new lock to drm_mm as at least for i915, we want to use a similar per-vm lock (of which the drm_mm is just one part). -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 v2 2/4] drm/i915: implement set_ncts callback
On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> From: Libin Yang
>
> Display 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/i915_reg.h| 2 +
> drivers/gpu/drm/i915/intel_audio.c | 95
> ++
> 2 files changed, 97 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ea46d68..da2d128 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7014,6 +7014,8 @@ enum skl_disp_power_wells {
> _HSW_AUD_MISC_CTRL_A, \
> _HSW_AUD_MISC_CTRL_B)
>
> +#define HSW_AUD_PIPE_CONN_SEL_CTRL 0x650ac
You're not using this register in this patch.
> +
> #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, \
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index dc32cf4..eddf37f 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -68,6 +68,30 @@ static const struct {
> { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> };
>
> +#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)
> {
> @@ -514,12 +538,83 @@ static int i915_audio_component_get_cdclk_freq(struct
> device *dev)
> return ret;
> }
>
> +static int i915_audio_component_set_ncts(struct device *dev, int port,
> + int dev_entry, 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 i;
> + int n_low, n_up, n = 0;
> +
> + /* 1. get the pipe */
> + for_each_intel_encoder(drm_dev, intel_encoder) {
> + 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 == -1) {
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. Need set the N/CTS manually at some frequencies */
> + if ((mode->clock != TMDS_297M) &&
> + (mode->clock != TMDS_296M)) {
> + tmp = I915_READ(HSW_AUD_CFG(pipe));
> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> + I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> + return 0;
> + }
I'm thinking it would be better to always use the manual setting. There
may be obscure bugs due to *sometimes* using automatic mode and some
other times not. Or maybe we could use automatic mode in error
scenarios?
> +
> + /* 3. calculate the N/CTS */
> + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> + if ((rate == aud_ncts[i].sample_rate) &&
> + (mode->clock == aud_ncts[i].clock)) {
> + n = aud_ncts[i].n;
> + break;
> + }
> + }
This part looks like a function to be abstracted (see
e.g. audio_config_hdmi_pixel_clock).
> + if (n == 0)
> + return 0;
Re: [Intel-gfx] [PATCH 00/18] dev->struct_mutex crusade
On Mon, Aug 10, 2015 at 12:53:23PM +0100, Chris Wilson wrote: > On Mon, Aug 10, 2015 at 01:35:58PM +0200, Thierry Reding wrote: > > On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote: > > > Hi all, > > > > > > I wanted to take another look at struct_mutex usage in modern (gem) > > > drivers and > > > noticed that for a fair lot we're very to be completely struct_mutex free. > > > > > > This pile here is the the simple part, which mostly just removes code and > > > mutex_lock/unlock calls. All the patches here are independent and can be > > > merged > > > in any order whatsoever. My plan is to send out a pull request for all > > > those not > > > picked up by driver maintainers in 2-3 weeks or so, assuming no one > > > complains. > > > > > > Of course review & comments still very much welcome. > > > > > > The more tricky 2nd part of this (and that one's not yet done) is to > > > rework the > > > gem mmap handler to use the same kref_get_unless_zero trick as ttm. With > > > that > > > there's no core requirement to hold struct_mutex over the final unref, > > > which > > > means we can make that one lockless. I plan to add a > > > gem_object_free_unlocked > > > for all the drivers which don't have any need for this lock. > > > > > > Also there's a few more drivers which can be made struct_mutex free > > > easily, I'll > > > propably stitch together poc patches for those. > > > > There's a concurrency bug in Tegra DRM currently because we don't lock > > accesses to drm_mm (I guess this demonstrates how badly we need better > > testing...) and it seems like this is typically protected by the very > > same struct_mutex that you're on a crusade to remove. If your goal is > > to get rid of it for good, should we simply add a separate lock just > > for the drm_mm? We don't have another one that would fit. > > Actually that is one of the first targets for more fine-grained locking. > I would not add a new lock to drm_mm as at least for i915, we want to use > a similar per-vm lock (of which the drm_mm is just one part). Sorry if I was being unclear. I wasn't suggesting adding the lock to struct drm_mm, but rather add a driver-specific one specifically to serialize accesses to the drm_mm. I agree that it's better to do this in a driver-specific way because other structures may need to be protected by the same lock. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/edid: Use ARRAY_SIZE in drm_add_modes_noedid
On Mon, Aug 10, 2015 at 11:55:37AM +0200, Daniel Vetter wrote: > Spotted while reading code for random reasons. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_edid.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 4a403eb90ded..4780b1924bef 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -3810,7 +3810,7 @@ int drm_add_modes_noedid(struct drm_connector > *connector, > struct drm_display_mode *mode; > struct drm_device *dev = connector->dev; > > - count = sizeof(drm_dmt_modes) / sizeof(struct drm_display_mode); > + count = ARRAY_SIZE(drm_dmt_modes); > if (hdisplay < 0) > hdisplay = 0; > if (vdisplay < 0) Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/doc: Update docs about device instance setup
On Mon, Aug 10, 2015 at 11:55:38AM +0200, Daniel Vetter wrote: > ->load is depracated, bus functionst are deprecated and everyone > should use drm_dev_alloc®ister. Why would you want to deprecated ->load()? Even if you use drm_dev_alloc() and drm_dev_register(), there's still use for ->load() because it gives you the subsystem-level initialization entry point. Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/18] dev->struct_mutex crusade
On Mon, Aug 10, 2015 at 01:35:58PM +0200, Thierry Reding wrote: > On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote: > > Hi all, > > > > I wanted to take another look at struct_mutex usage in modern (gem) drivers > > and > > noticed that for a fair lot we're very to be completely struct_mutex free. > > > > This pile here is the the simple part, which mostly just removes code and > > mutex_lock/unlock calls. All the patches here are independent and can be > > merged > > in any order whatsoever. My plan is to send out a pull request for all > > those not > > picked up by driver maintainers in 2-3 weeks or so, assuming no one > > complains. > > > > Of course review & comments still very much welcome. > > > > The more tricky 2nd part of this (and that one's not yet done) is to rework > > the > > gem mmap handler to use the same kref_get_unless_zero trick as ttm. With > > that > > there's no core requirement to hold struct_mutex over the final unref, which > > means we can make that one lockless. I plan to add a > > gem_object_free_unlocked > > for all the drivers which don't have any need for this lock. > > > > Also there's a few more drivers which can be made struct_mutex free easily, > > I'll > > propably stitch together poc patches for those. > > There's a concurrency bug in Tegra DRM currently because we don't lock > accesses to drm_mm (I guess this demonstrates how badly we need better > testing...) and it seems like this is typically protected by the very > same struct_mutex that you're on a crusade to remove. If your goal is > to get rid of it for good, should we simply add a separate lock just > for the drm_mm? We don't have another one that would fit. Yes, please just add your own driver-private lock. The next struct_mutex crusade series will actually do exactly that, at least for simple cases like armada. With that changes most drivers don't care about struct_mutex any more in their ->gem_free_object hook and I plan to add a new ->gem_free_object_unlocked variant for all the drivers which don't have any residual usage of struct_mutex. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 v3 3/4] ALSA: hda - display audio call ncts callback
On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> From: Libin Yang
>
> On some Intel platforms, display audio need set N/CTS
> manually at some TMDS frequencies.
>
> Signed-off-by: Libin Yang
> ---
> sound/pci/hda/patch_hdmi.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index a97db5f..918435e 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,17 @@ static int generic_hdmi_playback_pcm_prepare(struct
> hda_pcm_stream *hinfo,
> intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx);
> }
>
> + /* Set N/CTS for HSW, BDW and SKL platforms.
> + * These platforms need call set_ncts to set the N/CTS manually,
> + * otherwise there is no sound in some sample rates.
> + */
> + if (is_haswell_plus(codec)) {
Maybe you should move this check to the graphics side, and do the call
whenever the callback is non-NULL, and you can be sure of the pin->port
mapping?
> + /* Todo: add DP1.2 MST audio support later */
> + if (acomp && acomp->ops && acomp->ops->set_ncts)
> + acomp->ops->set_ncts(acomp->dev,
> + intel_pin2port(pin_nid),
> + 0, 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
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> From: Libin Yang
>
> When modeset occurs and the TMDS frequency is set to some
> speical value, the N/CTS need to be set manually if audio
> is playing.
When modeset occurs, we disable everything, and then re-enable
everything, and notify the audio driver every step of the way. IIUC
there should be no audio playing across the modeset, and when the
modeset is complete and we've set the valid ELD, the audio driver should
call the callback from the earlier patches to set the correct audio
rate.
Why is this patch needed? Please enlighten me.
Also some comments in-line, provided you've convinced me first. ;)
> Signed-off-by: Libin Yang
> ---
> drivers/gpu/drm/i915/i915_reg.h| 6 ++
> drivers/gpu/drm/i915/intel_audio.c | 42
> ++
> 2 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index da2d128..85f3beb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7030,6 +7030,12 @@ enum skl_disp_power_wells {
> _HSW_AUD_DIG_CNVT_2)
> #define DIP_PORT_SEL_MASK0x3
>
> +#define _HSW_AUD_STR_DESC_1 0x65084
> +#define _HSW_AUD_STR_DESC_2 0x65184
> +#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 eddf37f..082f96d 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -235,6 +235,9 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> const uint8_t *eld = connector->eld;
> uint32_t tmp;
> int len, i;
> + int cvt_idx;
> + int n_low, n_up, n;
> + int base_rate, mul, div, rate;
>
> DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
> pipe_name(pipe), drm_eld_size(eld));
> @@ -267,6 +270,21 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> tmp |= AUDIO_ELD_VALID(pipe);
> I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
>
> + if ((mode->clock == 297000) ||
> + (mode->clock == DIV_ROUND_UP(297000 * 1000, 1001))) {
> + 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;
> + }
This should be abstracted to a separate function.
> +
> /* Enable timestamps */
> tmp = I915_READ(HSW_AUD_CFG(pipe));
> tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> @@ -276,6 +294,30 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> tmp |= AUD_CONFIG_N_VALUE_INDEX;
> else
> tmp |= audio_config_hdmi_pixel_clock(mode);
> +
> + if ((mode->clock != 297000) &&
> + (mode->clock != DIV_ROUND_UP(297000 * 1000, 1001))) {
> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> + } else {
> + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> + if ((rate == aud_ncts[i].sample_rate) &&
> + (mode->clock == aud_ncts[i].clock)) {
> + n = aud_ncts[i].n;
> + break;
> + }
> + }
> + if (n != 0) {
> + tmp |= AUD_CONFIG_N_PROG_ENABLE;
> + n_low = n & 0xfff;
> + n_up = (n >> 12) & 0xff;
> + tmp |= AUD_CONFIG_N_PROG_ENABLE;
> + tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> + tmp |= (n_up << AUD_CONFIG_UPPER_N_SHIFT);
> + tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> + tmp |= (n_low << AUD_CONFIG_LOWER_N_SHIFT);
> + }
> + }
Please de-duplicate the copy-paste from patch 2. At the very least you
should use a helper for finding the parameters from the table.
> +
> 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
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lis
Re: [Intel-gfx] [PATCH 2/2] drm/doc: Update docs about device instance setup
On Mon, Aug 10, 2015 at 01:59:07PM +0200, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 11:55:38AM +0200, Daniel Vetter wrote: > > ->load is depracated, bus functionst are deprecated and everyone > > should use drm_dev_alloc®ister. > > Why would you want to deprecated ->load()? Even if you use > drm_dev_alloc() and drm_dev_register(), there's still use for ->load() > because it gives you the subsystem-level initialization entry point. ->load is called after the drm /dev node is registered and hence you can't really do any driver setup in there without risking races. We paper over that using drm_global_mutex, but that doesn't work for any other driver/userspace interface like sysfs/debugfs because of deadlocks. And we can't just reorder ->load to happen before the /dev nodes are registered because a lot of drivers would fall over if we do that. This is typical midlayer fail where the core calls into the driver instead of the other way round. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 1/2] drm/edid: Use ARRAY_SIZE in drm_add_modes_noedid
On Mon, Aug 10, 2015 at 01:57:21PM +0200, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 11:55:37AM +0200, Daniel Vetter wrote: > > Spotted while reading code for random reasons. > > > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/drm_edid.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 4a403eb90ded..4780b1924bef 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -3810,7 +3810,7 @@ int drm_add_modes_noedid(struct drm_connector > > *connector, > > struct drm_display_mode *mode; > > struct drm_device *dev = connector->dev; > > > > - count = sizeof(drm_dmt_modes) / sizeof(struct drm_display_mode); > > + count = ARRAY_SIZE(drm_dmt_modes); > > if (hdisplay < 0) > > hdisplay = 0; > > if (vdisplay < 0) > > > Reviewed-by: Thierry Reding Thanks for the review, applied to drm-misc. -Danie -- Daniel Vetter Software Engineer, Intel Corporation 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] drm/i915: Use CONFIG_DRM_FBDEV_EMULATION
On Mon, Aug 10, 2015 at 01:48:53PM +0200, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 01:34:08PM +0200, Daniel Vetter wrote: > > Instead of our own duplicated one. This fixes a bug in the driver > > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > > try to unregister the nonexistent fbdev drm_framebuffer. > > > > Cc: Archit Taneja > > Cc: Maarten Lankhorst > > Reported-by: Maarten Lankhorst > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/i915/Kconfig | 15 --- > > drivers/gpu/drm/i915/Makefile| 2 +- > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > 7 files changed, 8 insertions(+), 23 deletions(-) > > Isn't this going to cause some pain to users because .config may not > have this symbol yet? Arguably this is somewhat mitigated by the fact > that both symbols are "default y", but technically somebody could have > DRM_I915_FBDEV=n in their .config and after this change fbdev emulation > will be switched on again. > > I'm not sure how to upgrade more sanely, though, so perhaps this is just > a bullet that needs biting. There are other drivers two with their private FBDEV option (like msm) so I don't think we can do any sensible upgrade logic that just works. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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] drm/atomic: Paper over locking WARN in default_state_clear
On Fri, Jul 31, 2015 at 03:41:15PM +0200, Daniel Vetter wrote: > On Fri, Jul 31, 2015 at 10:34:43AM +0200, Maarten Lankhorst wrote: > > Hey, > > > > Op 29-07-15 om 12:51 schreef Daniel Vetter: > > > In > > > > > > commit 6f75cea66c8dd043ced282016b21a639af176642 > > > Author: Daniel Vetter > > > Date: Wed Nov 19 18:38:07 2014 +0100 > > > > > > drm/atomic: Only destroy connector states with connection mutex held > > > > > > I tried to fix races of atomic commits against connector > > > hot-unplugging. The idea is to ensure lifetimes by holding the > > > connection_mutex long enough. That works for synchronous commits, but > > > not for async ones. > > > > > > For async atomic commit we really need to fix up connector lifetimes > > > for real. But that's a much bigger task, so just add more duct-tape: > > > For cleaning up connector states we currently don't need the connector > > > itself. So NULL it out and remove the locking check. Of course that > > > check was to protect the entire sequence, but the modeset itself > > > should be save since currently DP MST hot-removal does a dpms-off. And > > > that should synchronize with any outstanding async atomic commit. > > > > > > Or at least that's my hope, this is all a giant mess. > > > > > > Reported-by: Maarten Lankhorst > > > Cc: Maarten Lankhorst > > > Signed-off-by: Daniel Vetter > > > --- > > > drivers/gpu/drm/drm_atomic.c | 12 +--- > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > > index 3efd91c0c6cb..434915448ea0 100644 > > > --- a/drivers/gpu/drm/drm_atomic.c > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > @@ -153,9 +153,15 @@ void drm_atomic_state_default_clear(struct > > > drm_atomic_state *state) > > > if (!connector) > > > continue; > > > > > > - WARN_ON(!drm_modeset_is_locked(&config->connection_mutex)); > > > - > > > - connector->funcs->atomic_destroy_state(connector, > > > + /* > > > + * FIXME: Async commits can race with connector unplugging and > > > + * there's currently nothing that prevents cleanup up state for > > > + * deleted connectors. As long as the callback doesn't look at > > > + * the connector we'll be fine though, so make sure that's the > > > + * case by setting all connector pointers to NULL. > > > + */ > > > + state->connector_states[i]->connector = NULL; > > > + connector->funcs->atomic_destroy_state(NULL, > > > > > > state->connector_states[i]); > > > > > This wouldn't provide any additional guarantee during the async commit > > itself, so please don't do this. :-) > > Nope, it's really just a big reminder that we have a bug here. Ok, picked up to drm-misc with Maarten's irc r-b. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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] drm/atomic: Call ww_acquire_done after check phase is complete
On Thu, Aug 06, 2015 at 03:06:40PM +0200, Daniel Vetter wrote: > We want to make sure that no one tries to acquire more locks and > states, and ww mutexes provide debug facilities for that. So use them. > > v2: Only call acquire_done when ->atomic_check was successful to avoid > falling over an -EDEADLK (spotted by Maarten). > > Cc: Rob Clark > Cc: Maarten Lankhorst > Signed-off-by: Daniel Vetter Picked up to drm-misc with Maarten's irc r-b. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 1/3] drm/i915: Dont enable hpd for eDP
On Mon, 10 Aug 2015, Sivakumar Thulasimani
wrote:
> On 8/10/2015 5:07 PM, Jani Nikula wrote:
>> On Mon, 10 Aug 2015, Sivakumar Thulasimani
>> wrote:
>>> Reviewed-by: Sivakumar Thulasimani
>>>
>>> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
With HPD support added for all ports including PORT_A, setting hpd_pin will
result in enabling of hpd to edp as well. There is no need to enable HPD on
PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
be connected. it can be added back when required
>> What? You can't just go ahead and remove HPD from eDP sinks.
>>
>> BR,
>> Jani.
> Nope, we are not removing HPD for edp sinks, it was never there in the
> first place. It was
> enabled for CHV (even there by mistake since PORT B/C was both DP and
> eDP) but it was
> never there for any other plaforms nor is it used for any purpose (PSR
> must use it, but i
> dont see code for it as well).
Are you saying there's no HPD enabled in our *hardware* for eDP? Or
driver?
My point is, is this patch making it harder to enable eDP hpd handling
(e.g. for PSR or DP link re-training) in the future? We currently take
it into account in a few places, and if we start removing that, it will
be a loss of effort to first remove and then add it back.
BR,
Jani.
>>
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/intel_dp.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index fcc64e5..5a614c9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
/* Set up the hotplug pin. */
switch (port) {
case PORT_A:
- intel_encoder->hpd_pin = HPD_PORT_A;
+ /* Not enabling edp interrupts */
break;
case PORT_B:
intel_encoder->hpd_pin = HPD_PORT_B;
>>> --
>>> regards,
>>> Sivakumar
>>>
>>> ___
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> 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
Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: implement set_ncts callback
On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> From: Libin Yang
>
> Display 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/i915_reg.h| 2 +
> drivers/gpu/drm/i915/intel_audio.c | 95
> ++
> 2 files changed, 97 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ea46d68..da2d128 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7014,6 +7014,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, \
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index dc32cf4..eddf37f 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -68,6 +68,30 @@ static const struct {
> { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> };
>
> +#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)
> {
> @@ -514,12 +538,83 @@ static int i915_audio_component_get_cdclk_freq(struct
> device *dev)
> return ret;
> }
>
> +static int i915_audio_component_set_ncts(struct device *dev, int port,
> + int dev_entry, 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 i;
> + int n_low, n_up, n = 0;
I think you'll need the power well get here, and put at the end. Or
check that we it.
> +
> + /* 1. get the pipe */
> + for_each_intel_encoder(drm_dev, intel_encoder) {
> + 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 == -1) {
> + 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. Need set the N/CTS manually at some frequencies */
> + if ((mode->clock != TMDS_297M) &&
> + (mode->clock != TMDS_296M)) {
> + tmp = I915_READ(HSW_AUD_CFG(pipe));
> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> + I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> + return 0;
> + }
> +
> + /* 3. calculate the N/CTS */
> + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> + if ((rate == aud_ncts[i].sample_rate) &&
> + (mode->clock == aud_ncts[i].clock)) {
> + n = aud_ncts[i].n;
> + break;
> + }
> + }
> + if (n == 0)
> + return 0;
> + n_low = n & 0xfff;
> + n_up = (n >> 12) & 0xff;
> +
> + /* 4. set the N/CTS */
> + tmp = I915_READ(HSW_AUD_CFG(pipe));
> + tmp |= AUD_CONFIG_N_PROG_ENABLE;
> + tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> + tmp |= (n_up << AUD_CONFIG_UPPER_N_SHIFT);
> + tmp &=
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Dont enable hpd for eDP
On Mon, Aug 10, 2015 at 03:14:36PM +0300, Jani Nikula wrote: > On Mon, 10 Aug 2015, Sivakumar Thulasimani > wrote: > > On 8/10/2015 5:07 PM, Jani Nikula wrote: > >> On Mon, 10 Aug 2015, Sivakumar Thulasimani > >> wrote: > >>> Reviewed-by: Sivakumar Thulasimani > >>> > >>> On 8/10/2015 10:35 AM, Sonika Jindal wrote: > With HPD support added for all ports including PORT_A, setting hpd_pin > will > result in enabling of hpd to edp as well. There is no need to enable HPD > on > PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will > be connected. it can be added back when required > >> What? You can't just go ahead and remove HPD from eDP sinks. > >> > >> BR, > >> Jani. > > Nope, we are not removing HPD for edp sinks, it was never there in the > > first place. It was > > enabled for CHV (even there by mistake since PORT B/C was both DP and > > eDP) but it was > > never there for any other plaforms nor is it used for any purpose (PSR > > must use it, but i > > dont see code for it as well). > > Are you saying there's no HPD enabled in our *hardware* for eDP? Or > driver? > > My point is, is this patch making it harder to enable eDP hpd handling > (e.g. for PSR or DP link re-training) in the future? We currently take > it into account in a few places, and if we start removing that, it will > be a loss of effort to first remove and then add it back. I have a branch with (untested) port A/E HPD support (+ a bunch of SPT+ irq handler reorganization). I guess I should post that in case people are interested in this stuff. -- 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 1/3] drm/i915: Dont enable hpd for eDP
On 8/10/2015 5:44 PM, Jani Nikula wrote:
On Mon, 10 Aug 2015, Sivakumar Thulasimani
wrote:
On 8/10/2015 5:07 PM, Jani Nikula wrote:
On Mon, 10 Aug 2015, Sivakumar Thulasimani
wrote:
Reviewed-by: Sivakumar Thulasimani
On 8/10/2015 10:35 AM, Sonika Jindal wrote:
With HPD support added for all ports including PORT_A, setting hpd_pin will
result in enabling of hpd to edp as well. There is no need to enable HPD on
PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
be connected. it can be added back when required
What? You can't just go ahead and remove HPD from eDP sinks.
BR,
Jani.
Nope, we are not removing HPD for edp sinks, it was never there in the
first place. It was
enabled for CHV (even there by mistake since PORT B/C was both DP and
eDP) but it was
never there for any other plaforms nor is it used for any purpose (PSR
must use it, but i
dont see code for it as well).
Are you saying there's no HPD enabled in our *hardware* for eDP? Or
driver?
My point is, is this patch making it harder to enable eDP hpd handling
(e.g. for PSR or DP link re-training) in the future? We currently take
it into account in a few places, and if we start removing that, it will
be a loss of effort to first remove and then add it back.
BR,
Jani.
i was referring to our driver only.
Our VLV/CHV code already receives hpd for every pps on and off which is
later ignored. if we dont disable HPD on eDPs this behavior will be extended
for all platforms which i feel is too costly to keep enabled when there
is no
purpose for it right now.
if anyone later requires HPD , they might need it for PSR panels only and
since that code is not available as of now, this can be added along with
those
changes so i feel this is the better tradeoff :)
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/intel_dp.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fcc64e5..5a614c9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5784,7 +5784,7 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
/* Set up the hotplug pin. */
switch (port) {
case PORT_A:
- intel_encoder->hpd_pin = HPD_PORT_A;
+ /* Not enabling edp interrupts */
break;
case PORT_B:
intel_encoder->hpd_pin = HPD_PORT_B;
--
regards,
Sivakumar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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/2] drm/doc: Update docs about device instance setup
On Mon, Aug 10, 2015 at 02:07:49PM +0200, Daniel Vetter wrote: > On Mon, Aug 10, 2015 at 01:59:07PM +0200, Thierry Reding wrote: > > On Mon, Aug 10, 2015 at 11:55:38AM +0200, Daniel Vetter wrote: > > > ->load is depracated, bus functionst are deprecated and everyone > > > should use drm_dev_alloc®ister. > > > > Why would you want to deprecated ->load()? Even if you use > > drm_dev_alloc() and drm_dev_register(), there's still use for ->load() > > because it gives you the subsystem-level initialization entry point. > > ->load is called after the drm /dev node is registered and hence you can't > really do any driver setup in there without risking races. We paper over > that using drm_global_mutex, but that doesn't work for any other > driver/userspace interface like sysfs/debugfs because of deadlocks. > > And we can't just reorder ->load to happen before the /dev nodes are > registered because a lot of drivers would fall over if we do that. > > This is typical midlayer fail where the core calls into the driver instead > of the other way round. Okay, but then if we're going to deprecate ->load(), I think we should also come up with an upgrade plan. As it is, this just says that users shouldn't do ->load(), but it doesn't tell them what to do instead. Would the proper procedure be to fix drivers so that ->load() can be called between drm_dev_alloc() and drm_dev_register()? I suppose we could add some sort of (temporary) flag for drivers to signal this and then have drm_dev_register() call ->load() at the right time. If drivers don't support it, we can keep what we have. That, of course, doesn't get rid of the midlayer, so perhaps a better way forward would be to tell driver writers that they should be doing subsystem-level setup between drm_dev_alloc() and drm_dev_register(). Thierry signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 11/11] drm/i915: Max DOT clock frequency to debugfs
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6910 -Summary- Platform Delta drm-intel-nightly Series Applied ILK -2 302/302 300/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT 283/283 283/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_flip@flip-vs-dpms-interruptible PASS(1) DMESG_WARN(1) *ILK igt@kms_flip@wf_vblank-vs-modeset-interruptible PASS(1) DMESG_WARN(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests
On Thu, Aug 06, 2015 at 02:33:31PM -0700, Jesse Barnes wrote: > On 08/06/2015 02:30 PM, Daniel Vetter wrote: > > On Fri, May 29, 2015 at 09:52:52AM +0200, Daniel Vetter wrote: > >> On Thu, May 28, 2015 at 05:53:17PM +0300, David Weinehall wrote: > >>> On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote: > A simple functional test here which does: > a) an execbuf with just 1 batch. With full ppgtt you should get that one > at offset 0. If not, skip the testcase. > b) set the NO_ZEROMAP property. > c) re-run the same batch, assert that now the buffer is relocated to > something non-0. > > Just to make sure we have a bare minimal testcase to make sure we don't > break this. > >>> > >>> Maybe this should be added to another test rather than here? This test > >>> is described as a: > >>> > >>> "Basic test for context set/get param input validation." > >>> > >>> Somehow I feel that testing whether the *functionality* is correct > >>> does not belong in this test, but rather in some test case that's > >>> already related to execbufs, or even a dedicated test case. > >>> > >>> But that might be over-engineering. Opinions? > >> > >> Yeah separate testcase would fit better, agreed. > > > > Update version of this patch is still missing. I'll need to revert the > > kernel side if this one doesn't show up soonish. > > > > Also you're breaking the invalid-flags testcase (did you bother to run > > them all and check for regressions?) which Jesse spotted, and with the new > > basic set this will be a P1 "I'm going to block everything" bug. > > We really need man pages for new ioctls as well in libdrm. Hmmm, this isn't a new ioctl, just a context parameter that can be set/queried using a pre-existing ioctl, but I can modify the existing manual page (if there is one?) to include information about the new parameter. Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests
On Thu, Aug 06, 2015 at 11:30:00PM +0200, Daniel Vetter wrote: [snip] > Update version of this patch is still missing. I'll need to revert the > kernel side if this one doesn't show up soonish. > > Also you're breaking the invalid-flags testcase (did you bother to run > them all and check for regressions?) which Jesse spotted, and with the new > basic set this will be a P1 "I'm going to block everything" bug. The patch I sent this Friday (the one I'd forgotten to send earlier) should work for the invalid-flags case, yes. Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/doc: Update docs about device instance setup
On Mon, Aug 10, 2015 at 02:34:18PM +0200, Thierry Reding wrote:
> On Mon, Aug 10, 2015 at 02:07:49PM +0200, Daniel Vetter wrote:
> > On Mon, Aug 10, 2015 at 01:59:07PM +0200, Thierry Reding wrote:
> > > On Mon, Aug 10, 2015 at 11:55:38AM +0200, Daniel Vetter wrote:
> > > > ->load is depracated, bus functionst are deprecated and everyone
> > > > should use drm_dev_alloc®ister.
> > >
> > > Why would you want to deprecated ->load()? Even if you use
> > > drm_dev_alloc() and drm_dev_register(), there's still use for ->load()
> > > because it gives you the subsystem-level initialization entry point.
> >
> > ->load is called after the drm /dev node is registered and hence you can't
> > really do any driver setup in there without risking races. We paper over
> > that using drm_global_mutex, but that doesn't work for any other
> > driver/userspace interface like sysfs/debugfs because of deadlocks.
> >
> > And we can't just reorder ->load to happen before the /dev nodes are
> > registered because a lot of drivers would fall over if we do that.
> >
> > This is typical midlayer fail where the core calls into the driver instead
> > of the other way round.
>
> Okay, but then if we're going to deprecate ->load(), I think we should
> also come up with an upgrade plan. As it is, this just says that users
> shouldn't do ->load(), but it doesn't tell them what to do instead.
>
> Would the proper procedure be to fix drivers so that ->load() can be
> called between drm_dev_alloc() and drm_dev_register()? I suppose we
> could add some sort of (temporary) flag for drivers to signal this and
> then have drm_dev_register() call ->load() at the right time. If drivers
> don't support it, we can keep what we have.
>
> That, of course, doesn't get rid of the midlayer, so perhaps a better
> way forward would be to tell driver writers that they should be doing
> subsystem-level setup between drm_dev_alloc() and drm_dev_register().
That's exactly what this patch tries to accomplish by updating the
kerneldoc and docbook. New sequence should be
device_probe_callback_or_whatever()
{
drm_dev_alloc();
... driver init code ...
drm_dev_register();
return 0;
}
Unfortunately the kerneldoc markdown isn't merged yet, otherwise I'd have
added this code snippet to the docs too.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
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 8/9] drm/i915: Implement WaPixelRepeatModeFixForC0:chv
On Mon, Jul 13, 2015 at 11:44:44AM +0530, Sivakumar Thulasimani wrote:
>
>
> On 6/29/2015 5:55 PM, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > DPLL_MD(PIPE_C) is AWOL on CHV. Instead of fixing it someone added
> > chicken bits to propagate the pixel multiplier from DPLL_MD(PIPE_B)
> > to either pipe B or C. So do that to make pixel repeat work on pipes
> > B and C. Pipe A is fine without any tricks.
> >
> > Fortunately the pixel repeat propagation appears to be a oneshot
> > operation, so once the value has been written we can clear the
> > chicken bits. So it is still possible to drive pipe B and C with
> > different pixel multipliers simultaneosly.
> >
> > Looks like DPLL_VGA_MODE_DIS must also be set in DPLL(PIPE_B)
> > for this to work. But since we keep that bit always set in all
> > DPLLs there's no problem.
> >
> > This of course means we can't reliably read out the pixel multiplier
> > for pipes B and C. That would make the state checker unhappy, so I
> > added shadow copies of those registers in to dev_priv. The other
> > option would have been to skip pixel multiplier, dpll_md an dotclock
> > checks entirely on CHV, but that feels like a serious loss of cross
> > checking, so just pretending that we have working DPLL MD registers
> > seemed better. Obviously with the shadow copies we can't detect if
> > the pixel multiplier was properly configured, nor can we take over
> > its state from the BIOS, but hopefully people won't have displays
> > that would be limitd to such crappy modes.
> >
> > There is one strange flicker still remaining. It's visible on
> > pipe C/HDMID when HDMIB is enabled while driven by pipe B.
> > It doesn't occur if pipe A drives HDMIB, nor is there any glitch
> > on pipe B/HDMIB when port C/HDMID starts up. I don't have a board
> > with HDMIC so not sure if it happens there too. So I'm not sure
> > if it's somehow tied in with this strange linkage between pipe B
> > and C. Sadly I was unable to find an enable sequence that would
> > avoid the glitch, but at least it's not fatal ie. the output
> > recovers afterwards.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 7 +++
> > drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > drivers/gpu/drm/i915/intel_display.c | 30 ++
> > 3 files changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 37cc653..adaa656 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1851,7 +1851,14 @@ struct drm_i915_private {
> >
> > u32 fdi_rx_config;
> >
> > + /* Shadow for DISPLAY_PHY_CONTROL which can't be safely read */
> > u32 chv_phy_control;
> > + /*
> > +* Shadows for CHV DPLL_MD regs to keep the state
> > +* checker somewhat working in the presence hardware
> > +* crappiness (can't read out DPLL_MD for pipes B & C).
> > +*/
> > + u32 chv_dpll_md[I915_MAX_PIPES];
> >
> > u32 suspend_count;
> > struct i915_suspend_saved_registers regfile;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index f08f729..2361347 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4580,6 +4580,9 @@ enum skl_disp_power_wells {
> >
> > #define CBR1_VLV (VLV_DISPLAY_BASE + 0x70400)
> > #define CBR_PND_DEADLINE_DISABLE (1<<31)
> > +#define CBR4_VLV (VLV_DISPLAY_BASE + 0x70450)
> > +#define CBR_DPLLBMD_PIPE_C(1<<29)
> > +#define CBR_DPLLBMD_PIPE_B(1<<18)
> >
> > /* FIFO watermark sizes etc */
> > #define G4X_FIFO_LINE_SIZE64
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index dec36a2..b862307 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1667,9 +1667,27 @@ static void chv_enable_pll(struct intel_crtc *crtc,
> > if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) ==
> > DPLL_LOCK_VLV), 1))
> > DRM_ERROR("PLL %d failed to lock\n", pipe);
> >
> > - /* not sure when this should be written */
> > - I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
> > - POSTING_READ(DPLL_MD(pipe));
> > + if (pipe != PIPE_A) {
> > + /*
> > +* WaPixelRepeatModeFixForC0:chv
> > +*
> > +* DPLLCMD is AWOL. Use chicken bits to propagate
> > +* the value from DPLLBMD to either pipe B or C.
> > +*/
> > + I915_WRITE(CBR4_VLV, pipe == PIPE_B ? CBR_DPLLBMD_PIPE_B :
> > CBR_DPLLBMD_PIPE_C);
> > + I915_WRITE(DPLL_MD(PIPE_B), pipe_config->dpll_hw_state.dpll_md);
> > + I915_WRITE(CBR4_VLV, 0);
> > + dev_priv->chv_dpll_md[pipe] =
> > pipe_config->dpll_hw_state.dpll_md;
> > +
Re: [Intel-gfx] [PATCH] drm/i915: Remove the failed context from the fpriv->context_idr
Chris Wilson writes: > If we encounter an allocation failure during ppggt creation (trivial > even with 16Gib+ RAM!), we need to remove the dead context from the > fpriv->context_idr along with the references. > > gem_exec_ctx: page allocation failure: order:0, mode:0x8004 > CPU: 3 PID: 27272 Comm: gem_exec_ctx Tainted: GW 4.2.0-rc5+ #37 > 880086ff7a78 816b947a 88041ed90038 > 8004 880086ff7b08 8114b1a5 880086ff7ac8 > 8108d848 81ce84b8 > Call Trace: > [] dump_stack+0x45/0x57 > [] warn_alloc_failed+0xd5/0x120 > [] ? __wake_up+0x48/0x60 > [] __alloc_pages_nodemask+0x73d/0x8e0 > [] ? i915_gem_execbuffer2+0x148/0x240 [i915] > [] __setup_page_dma+0x30/0x110 [i915] > [] gen8_ppgtt_init+0x31/0x2f0 [i915] > [] i915_ppgtt_init+0x30/0x80 [i915] > [] i915_ppgtt_create+0x48/0xc0 [i915] > [] i915_gem_create_context+0x1c2/0x390 [i915] > [] i915_gem_context_create_ioctl+0x5b/0xa0 [i915] > > leading to an oops in i915_gem_context_close. Also note that this > benchmark should not be running out of memory in the first place... > > Testcase: igt/benchmark/gem_exec_ctx -b create # ppgtt >= 2 > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/i915_gem_context.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > b/drivers/gpu/drm/i915/i915_gem_context.c > index d0f6d90f6d0e..d87e48a5 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -420,6 +420,7 @@ err_unpin: > if (vma) > i915_vma_unpin(vma); ^^ That would be nice to have. > err_destroy: > + idr_remove(&file_priv->context_idr, ctx->user_handle); > i915_gem_context_unreference(ctx); > return ERR_PTR(ret); This is not against drm-intel-next-queued. However in context of dinq: Reviewed-by: Mika Kuoppala > } > -- > 2.5.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Postpone plane readout until after encoder readout
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6911 -Summary- Platform Delta drm-intel-nightly Series Applied ILK -1 302/302 301/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT -2 283/283 281/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_flip@flip-vs-dpms-interruptible PASS(1) DMESG_WARN(1) *BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1) *BYT igt@gem_tiled_partial_pwrite_pread@reads PASS(1) FAIL(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: fix stolen bios_reserved checks
I started digging this when I noticed that the BDW code was just
reserving 1mb by coincidence since it was reading reserved fields.
Then I noticed we didn't have any values set for SNB and earlier, and
that the HSW sizes were wrong. After that, I noticed that the reserved
area has a specific start, and may not exactly end where the stolen
memory ends. I also noticed the base pointer can be zero. So I decided
to just write a single patch fixing everything instead of 20 patches
that would be much harder to review.
This patch may solve random stolen memory corruption/problems on
almost all platforms. Notice that since this is always dealing with
the top of the stolen memory, the problems are not so easy to
reproduce - especially since FBC is still disabled by default.
One of the major differences of this patch is that we now look at both
the size and base address. By only looking at the size we were
assuming that the reserved area was always at the very top of
stolen, which is not always true.
After we merge the patch series that allows user space to allocate
stolen memory we'll be able to write IGT tests that maybe catch the
bugs fixed by this patch.
v2:
- s/BIOS reserved/stolen reserved/g (Chris)
- Don't DRM_ERROR if we can't do anything about it (Chris)
- Improve debug messages (Chris).
- Use the gen7 version instead of gen6 on HSW. Tom found some
documentation problems, so I think with gen7 we're on the safer
side (Tom).
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_gem_stolen.c | 159 +
drivers/gpu/drm/i915/i915_reg.h| 23 +++--
2 files changed, 159 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index ed682a9..a36cb95 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -186,11 +186,103 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
drm_mm_takedown(&dev_priv->mm.stolen);
}
+static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
+unsigned long *base, unsigned long *size)
+{
+ uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+
+ *base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
+
+ switch (reg_val & GEN6_STOLEN_RESERVED_SIZE_MASK) {
+ case GEN6_STOLEN_RESERVED_1M:
+ *size = 1024 * 1024;
+ break;
+ case GEN6_STOLEN_RESERVED_512K:
+ *size = 512 * 1024;
+ break;
+ case GEN6_STOLEN_RESERVED_256K:
+ *size = 256 * 1024;
+ break;
+ case GEN6_STOLEN_RESERVED_128K:
+ *size = 128 * 1024;
+ break;
+ default:
+ *size = 1024 * 1024;
+ MISSING_CASE(reg_val & GEN6_STOLEN_RESERVED_SIZE_MASK);
+ }
+}
+
+static void gen7_get_stolen_reserved(struct drm_i915_private *dev_priv,
+unsigned long *base, unsigned long *size)
+{
+ uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+
+ *base = reg_val & GEN7_STOLEN_RESERVED_ADDR_MASK;
+
+ switch (reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK) {
+ case GEN7_STOLEN_RESERVED_1M:
+ *size = 1024 * 1024;
+ break;
+ case GEN7_STOLEN_RESERVED_256K:
+ *size = 256 * 1024;
+ break;
+ default:
+ *size = 1024 * 1024;
+ MISSING_CASE(reg_val & GEN7_STOLEN_RESERVED_SIZE_MASK);
+ }
+}
+
+static void gen8_get_stolen_reserved(struct drm_i915_private *dev_priv,
+unsigned long *base, unsigned long *size)
+{
+ uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+
+ *base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
+
+ switch (reg_val & GEN8_STOLEN_RESERVED_SIZE_MASK) {
+ case GEN8_STOLEN_RESERVED_1M:
+ *size = 1024 * 1024;
+ break;
+ case GEN8_STOLEN_RESERVED_2M:
+ *size = 2 * 1024 * 1024;
+ break;
+ case GEN8_STOLEN_RESERVED_4M:
+ *size = 4 * 1024 * 1024;
+ break;
+ case GEN8_STOLEN_RESERVED_8M:
+ *size = 8 * 1024 * 1024;
+ break;
+ default:
+ *size = 8 * 1024 * 1024;
+ MISSING_CASE(reg_val & GEN8_STOLEN_RESERVED_SIZE_MASK);
+ }
+}
+
+static void bdw_get_stolen_reserved(struct drm_i915_private *dev_priv,
+ unsigned long *base, unsigned long *size)
+{
+ uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
+ unsigned long stolen_top;
+
+ stolen_top = dev_priv->mm.stolen_base + dev_priv->gtt.stolen_size;
+
+ *base = reg_val & GEN6_STOLEN_RESERVED_ADDR_MASK;
+
+ /* On these platforms, the register doesn't have a size field, so the
+* size is the distance between the base and the top of the stole
Re: [Intel-gfx] [PATCH] drm/i915: fix stolen bios_reserved checks
On Mon, Aug 10, 2015 at 02:57:32PM -0300, Paulo Zanoni wrote: > I started digging this when I noticed that the BDW code was just > reserving 1mb by coincidence since it was reading reserved fields. > Then I noticed we didn't have any values set for SNB and earlier, and > that the HSW sizes were wrong. After that, I noticed that the reserved > area has a specific start, and may not exactly end where the stolen > memory ends. I also noticed the base pointer can be zero. So I decided > to just write a single patch fixing everything instead of 20 patches > that would be much harder to review. > > This patch may solve random stolen memory corruption/problems on > almost all platforms. Notice that since this is always dealing with > the top of the stolen memory, the problems are not so easy to > reproduce - especially since FBC is still disabled by default. > > One of the major differences of this patch is that we now look at both > the size and base address. By only looking at the size we were > assuming that the reserved area was always at the very top of > stolen, which is not always true. > > After we merge the patch series that allows user space to allocate > stolen memory we'll be able to write IGT tests that maybe catch the > bugs fixed by this patch. > > v2: > - s/BIOS reserved/stolen reserved/g (Chris) > - Don't DRM_ERROR if we can't do anything about it (Chris) > - Improve debug messages (Chris). > - Use the gen7 version instead of gen6 on HSW. Tom found some > documentation problems, so I think with gen7 we're on the safer > side (Tom). > > Signed-off-by: Paulo Zanoni Looks ok to me, I'd push for DRM_INFO() for the amount of memory available (since I think that is interesting as a user). Reviewed-by: Chris Wilson -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 1/4] drm/i915: Adding intel_panel_scale_none() helper function
I believe this function could be added along with the next patch that is
the first to use it...
Or it would be good to have a good commit message explaining why this
function is needed and what is be used for...
more bikeshedings inline:
On Mon, Aug 10, 2015 at 12:39 AM Xiong Zhang
wrote:
> Signed-off-by: Xiong Zhang
> ---
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_panel.c | 10 ++
> 2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 47cef0e..f57a0b4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1287,6 +1287,7 @@ int intel_panel_init(struct intel_panel *panel,
> void intel_panel_fini(struct intel_panel *panel);
> void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> struct drm_display_mode *adjusted_mode);
> +bool intel_panel_scale_none(struct intel_panel *panel);
> void intel_pch_panel_fitting(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config,
> int fitting_mode);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c
> b/drivers/gpu/drm/i915/intel_panel.c
> index e2ab3f6..4a573ac 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -46,6 +46,16 @@ intel_fixed_panel_mode(const struct drm_display_mode
> *fixed_mode,
> drm_mode_set_crtcinfo(adjusted_mode, 0);
> }
>
> +bool
> +intel_panel_scale_none(struct intel_panel *panel)
>
double negations always confuses me, when reading next patches it took few
seconds to realize on next patch that !scale_none was == fixed_mode...
but meh, I never have good suggestions to avoid double negations... so up
to you...
> +{
> + if (panel->fitting_mode == DRM_MODE_SCALE_NONE ||
> + panel->fixed_mode == NULL)
> + return true;
> + else
> + return false;
>
this could be just return (panel->fitting_mode == DRM_MODE_SCALE_NONE ||
panel->fixed_mode == NULL)
or ! if you remove the double negation...
> +}
> +
> /**
> * intel_find_panel_downclock - find the reduced downclock for LVDS in
> EDID
> * @dev: drm device
> --
> 1.8.2.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Treat foreign dma-buf imports as uncached
If the set of pages is being imported from another device, we cannot assume that it is fully coherent with the CPU cache, so mark it as such. However, if the source is the shared memory vgem allocator, we could treat the buffer as being cached (so long as all parties agree in the case the same buffer is shared between multiple devices) but as of today, vgem cannot export dma-bufs. Signed-off-by: Chris Wilson Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c index 7748d57adffd..da5e4fb02350 100644 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c @@ -265,8 +265,22 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev, i915_gem_object_init(obj, &i915_gem_object_dmabuf_ops); obj->base.import_attach = attach; + /* This bo is imported from a foreign HW device with which we cannot +* assume any coherency. Note that if this dma-buf was imported from +* a simple page allocator, like vgem, then we could treat this as +* cacheable (so long as all parties agree on that convention - i.e. +* if the same bo was shared with nouveau/radeon they also treat it +* as CPU cacheeable so that coherency is maintained between all +* parties). +*/ + ret = i915_gem_object_set_cache_level(obj, I915_CACHE_NONE); + if (ret) + goto fail_obj; + return &obj->base; +fail_obj: + drm_gem_object_unreference(&obj->base); fail_detach: dma_buf_detach(dma_buf, attach); dma_buf_put(dma_buf); -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6912 -Summary- Platform Delta drm-intel-nightly Series Applied ILK -1 302/302 301/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT -2 283/283 281/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_flip@flip-vs-dpms-interruptible PASS(1) DMESG_WARN(1) *BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1) *BYT igt@gem_persistent_relocs@interruptible PASS(1) FAIL(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH mesa v3] i965/gen8+: bo in state base address must be in 32-bit address range
On Mon, Aug 10, 2015 at 2:21 AM, Michel Thierry
wrote:
> Hi,
>
> Thanks for the comments,
>
> On 8/7/2015 11:46 PM, Kristian Høgsberg wrote:
>>
>> On Fri, Aug 7, 2015 at 2:45 AM, Michel Thierry
>> wrote:
>>>
>>> Gen8+ supports 48-bit virtual addresses, but some objects must always be
>>> allocated inside the 32-bit address range.
>>>
>>> In specific, any resource used with flat/heapless (0x-0xf000)
>>> General State Heap or Intruction State Heap must be in a 32-bit range
>>> (GSH / ISH), because the General State Offset and Instruction State
>>> Offset
>>> are limited to 32-bits.
>>
>>
>> This doesn't look right. From the workaround text, it doesn't sound
>> like this is a HW problem, it only affects GMM. In mesa, we don't use
>> "heapless" (which I assume means setting the base to 0 and max range)
>
>
> It is a hw problem, specifically in the state base address command. General
> State Base Address and Instruction Base Address shouldn't be > 4GB.
>
>> for instructions, dynamic state or surface state. Surface state and
>> dynamic state is always in our batch bo which is limited to 8k.
>> Provided STATE_BASE_ADDRESS works correctly in the HW, the batch bo
>> can be places anywhere in the aperture. Offsets to dynamic or surface
>> state is relative to the beginning of the batch bo and will always be
>> less than 4g. Instructions are in their own bo (brw->cache.bo), but
>> again, we point instruction state base to it and all our shader stage
>> setup code references the instructions relative to the beginning of
>> the instruction bo.
>
>
> I've seen the issue when the driver allocates mapped objects from bottom to
> top and the other bo's from top to bottom (gpu hangs). So I say this wa is
> needed.
mesa does use heapless for the scratch buffers, and on BDW+ that's
the only address type that's relative to general state base address.
So we need the workaround there, whether or not there's a HW
limitation/bug.
I don't think there is a HW problem though. The driver I'm working on
can use 48 bit ppgtt and Chris Wilson's softpin ioctl for userspace BO
placement. I enabled that and made the userspace allocator place all
BOs above 8g, and it all stilll worked. In particular, the
instructions were above 8g and referenced relative to instruction base
address. If the instruction base was limited to 4g, this would have
hung the GPU.
The fact that the HW limits the size of the heaps to 4g can be
restriction when we move to full 48 ppgtt, but for how mesa uses
STATE_BASE_ADDRESS, it only affects general state base address.
Kristiain
> -Michel
>
>>
>> Kristian
>>
>>> Use drm_intel_bo_emit_reloc_48bit when the 4GB limit is not necessary,
>>> and
>>> the bo can be in the full address space.
>>>
>>> This commit introduces a dependency of libdrm 2.4.63, which introduces
>>> the
>>> drm_intel_bo_emit_reloc_48bit function.
>>>
>>> v2: s/48baddress/48b_address/,
>>> Only use in OUT_RELOC64 cases, OUT_RELOC implies a 32-bit address
>>> offset
>>> is needed (Ben)
>>> v3: Added OUT_RELOC64_INSIDE_4G, so it stands out when a 64-bit
>>> relocation
>>> needs the 32-bit workaround (Chris)
>>>
>>> References:
>>> http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html
>>> Cc: Ben Widawsky
>>> Cc: Chris Wilson
>>> Signed-off-by: Michel Thierry
>>> ---
>>> configure.ac | 2 +-
>>> src/mesa/drivers/dri/i965/gen8_misc_state.c | 19 +++
>>> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 20
>>> src/mesa/drivers/dri/i965/intel_batchbuffer.h | 10 --
>>> 4 files changed, 36 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index af61aa2..c92ca44 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -68,7 +68,7 @@ AC_SUBST([OSMESA_VERSION])
>>> dnl Versions for external dependencies
>>> LIBDRM_REQUIRED=2.4.38
>>> LIBDRM_RADEON_REQUIRED=2.4.56
>>> -LIBDRM_INTEL_REQUIRED=2.4.60
>>> +LIBDRM_INTEL_REQUIRED=2.4.63
>>> LIBDRM_NVVIEUX_REQUIRED=2.4.33
>>> LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
>>> LIBDRM_FREEDRENO_REQUIRED=2.4.57
>>> diff --git a/src/mesa/drivers/dri/i965/gen8_misc_state.c
>>> b/src/mesa/drivers/dri/i965/gen8_misc_state.c
>>> index b20038e..73eba06 100644
>>> --- a/src/mesa/drivers/dri/i965/gen8_misc_state.c
>>> +++ b/src/mesa/drivers/dri/i965/gen8_misc_state.c
>>> @@ -28,6 +28,10 @@
>>>
>>> /**
>>>* Define the base addresses which some state is referenced from.
>>> + *
>>> + * Use OUT_RELOC64_INSIDE_4G instead of OUT_RELOC64, the General State
>>> + * Offset and Instruction State Offset are limited to 32-bits by
>>> hardware,
>>> + * and must be located in the first 4GBs (32-bit offset).
>>>*/
>>> void gen8_upload_state_base_address(struct brw_context *brw)
>>> {
>>> @@ -41,19 +45,18 @@ void gen8_upload_state_base_address(struct
>>> brw_context *brw)
>>> OUT_BATCH(0);
>>> OUT_BATCH(mocs_wb << 16);
>>> /* Sur
Re: [Intel-gfx] [PATCH] drm/i915/vlv: Add RPS debugfs disabling for vlv/chv
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6923 -Summary- Platform Delta drm-intel-nightly Series Applied ILK -2 302/302 300/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT -1 283/283 282/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_flip@flip-vs-dpms-interruptible PASS(1) DMESG_WARN(1) *ILK igt@kms_flip@wf_vblank-vs-modeset-interruptible PASS(1) DMESG_WARN(1) *BYT igt@gem_tiled_partial_pwrite_pread@reads PASS(1) FAIL(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 3/4] ALSA: hda - display audio call ncts callback
Hi Raymond,
From: Raymond Yau [mailto:superquad.vort...@gmail.com]
Sent: Monday, August 10, 2015 12:23 PM
To: Yang, Libin
Cc: alsa-de...@alsa-project.org; Takashi Iwai; Lin, Mengdong;
intel-gfx@lists.freedesktop.org
Subject: RE: [alsa-devel] [PATCH 3/4] ALSA: hda - display audio call ncts
callback
2015-8-10 上午11:15於 "Yang, Libin"
mailto:libin.y...@intel.com>>寫道：
>
> Hi Raymond,
>
> >
> > > > }
> > > >
> > > > + if (is_haswell_plus(codec)) {
> > > > + if (acomp && acomp->ops && acomp->ops->set_ncts)
> > > > + acomp->ops->set_ncts(acomp->dev, per_pin-
> > > >pin_nid - 4,
> > >
> > > Please describe more how "pin_nid - 4" is supposed to work. Also...
> >
> > OK, I see.
> >
> > >
> > > > + 0, runtime->rate);
> > >
> > > ... this implies that no MST support included?
> >
> > We will support MST later. Currently I just add the
> > interface to support MST, but not implementing it.
> Refer to DCN HDA040-A
> Multi-stream over Single Display Port
> Can the driver use subdevices for those display port support multi streaming ?
>
> [Libin] What do you mean subdevice here,
> using a struct device to represent a dev_entry or an int type?
http://git.kernel.org/cgit/linux/kernel/git/tiwai/hda-emu.git/tree/codecs/stac9227-intel-d946gzis-mobo?id=HEAD
When HDA codecs have three Audio Input widgets, the driver create three
subdevices for those desktop which have three or more input sources in the past
This is what we are thinking currently. Different companies
have different implementation. On currently Intel platforms,
it may show several pin widgets and each pin widget has
several device entry. But it actually only support 3 streams.
Mengdong is thinking to use dynamic PCM to implement it,
and so we don’t need each subdevice for each device entry.
I’m not sure we will use what solution. It seems it is a
good open question to discuss.
Regards,
Libin
ARECORD
List of CAPTURE Hardware Devices
card 0: Intel [HDA Intel], device 0: STAC92xx Analog [STAC92xx Analog]
Subdevices: 3/3
Subdevice #0: subdevice #0
Subdevice #1: subdevice #1
Subdevice #2: subdevice #2
With the auto generic parser , the driver create one subdevice for Analog two
subdevices for Alt Analog
List of CAPTURE Hardware Devices
card 0: Intel [HDA Intel], device 0: STAC92xx Analog [STAC92xx Analog]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: Intel [HDA Intel], device 2: STAC92xx Alt Analog [STAC92xx Alt Analog]
Subdevices: 2/2
Subdevice #0: subdevice #0
Subdevice #1: subdevice #1
>
> The specification allow up to 64 device entries
> This mean the number of subdevices is equal to the device list length
> More than one audio output /converters can be connected to the multi stream
> displayport pin widget but different device entry while only one audio output
> can be dynamically allocated to other HDMI pin widget
>
> [Libin] Yes, Pin widget can have multiple device entry and connecting
> different converters. The audio output will be based on device entry.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts callback
Hi Jani,
Thanks for reviewing, please see my comments
> -Original Message-
> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> Sent: Monday, August 10, 2015 7:46 PM
> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts
> callback
>
> On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> > From: Libin Yang
> >
> > Add the set_ncts callback.
> >
> > With the callback, audio driver can trigger
> > i915 driver to set the proper N/CTS
> > based on different sample rates.
> >
> > Signed-off-by: Libin Yang
> > ---
> > include/drm/i915_component.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/include/drm/i915_component.h
> b/include/drm/i915_component.h
> > index c9a8b64..7305881 100644
> > --- a/include/drm/i915_component.h
> > +++ b/include/drm/i915_component.h
> > @@ -33,6 +33,8 @@ struct i915_audio_component {
> > void (*put_power)(struct device *);
> > void (*codec_wake_override)(struct device *, bool
> enable);
> > int (*get_cdclk_freq)(struct device *);
> > + int (*set_ncts)(struct device *, int port, int dev_entry,
> > + int rate);
>
> I'd rather call this set_audio_rate or similar, and dropping the
> references to N and CTS. The caller does not need to know.
But it seems the set_audio_rate will confuse the user. From the
literal meaning, it is setting the rate of audio, such as sample rate,
which will make audio driver developers confused.
And the function is just setting N/CTS which is defined from
HDMI SPEC.
BTW: your comment reminds me that get_cdclk_freq() seems
to need change the name as cdclk is not in the open spec.
>
> I'm also not fond of adding a dev_entry parameter and leaving it
> unused. I do not think we know specifically how we're going to identify
> MST sinks, and the interface may need to be changed anyway. Let's
> force
> an update in the caller side as well when there's actually sensible
> support in our side.
The device entry is a concept in HDA spec. For driver implementation,
I think we can use an int variable or a struct device to represent it.
A int variable is an easy way. There is some place in audio driver using
int variable to represent the deivce entry. And audio driver will not
care how gfx driver supports the DP1.2 MST, I mean audio driver will
not know the structures inside gfx driver.
I will remove this parameter in this version if you are thinking the
MST interface is indeterminate.
BTW: do you know when gfx will normally support MST?
Best Regards,
Libin
>
> BR,
> Jani.
>
> > } *ops;
> > };
> >
> > --
> > 1.9.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: implement set_ncts callback
Hi Jani,
> -Original Message-
> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> Sent: Monday, August 10, 2015 8:00 PM
> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: implement set_ncts
> callback
>
> On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> > From: Libin Yang
> >
> > Display 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/i915_reg.h| 2 +
> > drivers/gpu/drm/i915/intel_audio.c | 95
> ++
> > 2 files changed, 97 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > index ea46d68..da2d128 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7014,6 +7014,8 @@ enum skl_disp_power_wells {
> > _HSW_AUD_MISC_CTRL_A, \
> > _HSW_AUD_MISC_CTRL_B)
> >
> > +#define HSW_AUD_PIPE_CONN_SEL_CTRL 0x650ac
>
> You're not using this register in this patch.
Oh, yes. In my first inside version, I use it. But remove using it
later. I will remove it. Thanks.
>
> > +
> > #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, \
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > index dc32cf4..eddf37f 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -68,6 +68,30 @@ static const struct {
> > { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> > };
> >
> > +#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)
> > {
> > @@ -514,12 +538,83 @@ static int
> i915_audio_component_get_cdclk_freq(struct device *dev)
> > return ret;
> > }
> >
> > +static int i915_audio_component_set_ncts(struct device *dev, int
> port,
> > + int dev_entry, 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 i;
> > + int n_low, n_up, n = 0;
> > +
> > + /* 1. get the pipe */
> > + for_each_intel_encoder(drm_dev, intel_encoder) {
> > + 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 == -1) {
>
> INVALID_PIPE
Got it. Thanks.
>
> > + 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. Need set the N/CTS manually at some frequencies */
> > + if ((mode->clock != TMDS_297M) &&
> > + (mode->clock != TMDS_296M)) {
> > + tmp = I915_READ(HSW_AUD_CFG(pipe));
> > + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > + I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > + return 0;
> > + }
>
> I'm thinking it would be better to always use the manual setting. There
> may be obsc
Re: [Intel-gfx] [PATCH v3 3/4] ALSA: hda - display audio call ncts callback
Hi Jani,
> -Original Message-
> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> Sent: Monday, August 10, 2015 8:03 PM
> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: Re: [Intel-gfx] [PATCH v3 3/4] ALSA: hda - display audio call
> ncts callback
>
> On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> > From: Libin Yang
> >
> > On some Intel platforms, display audio need set N/CTS
> > manually at some TMDS frequencies.
> >
> > Signed-off-by: Libin Yang
> > ---
> > sound/pci/hda/patch_hdmi.c | 23 +++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/sound/pci/hda/patch_hdmi.c
> b/sound/pci/hda/patch_hdmi.c
> > index a97db5f..918435e 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,17 @@ static int
> generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
> > intel_not_share_assigned_cvt(codec, pin_nid, per_pin-
> >mux_idx);
> > }
> >
> > + /* Set N/CTS for HSW, BDW and SKL platforms.
> > +* These platforms need call set_ncts to set the N/CTS manually,
> > +* otherwise there is no sound in some sample rates.
> > +*/
> > + if (is_haswell_plus(codec)) {
>
> Maybe you should move this check to the graphics side, and do the call
> whenever the callback is non-NULL, and you can be sure of the pin-
> >port
> mapping?
OK, I will remove this check here.
We have checked with silicon team for the mapping.
>
> > + /* Todo: add DP1.2 MST audio support later */
> > + if (acomp && acomp->ops && acomp->ops->set_ncts)
> > + acomp->ops->set_ncts(acomp->dev,
> > + intel_pin2port(pin_nid),
> > + 0, 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
>
> --
> Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: implement set_ncts callback
Hi Jani,
> -Original Message-
> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> Sent: Monday, August 10, 2015 8:17 PM
> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: implement set_ncts
> callback
>
> On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> > From: Libin Yang
> >
> > Display 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/i915_reg.h| 2 +
> > drivers/gpu/drm/i915/intel_audio.c | 95
> ++
> > 2 files changed, 97 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > index ea46d68..da2d128 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7014,6 +7014,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, \
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > index dc32cf4..eddf37f 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -68,6 +68,30 @@ static const struct {
> > { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> > };
> >
> > +#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)
> > {
> > @@ -514,12 +538,83 @@ static int
> i915_audio_component_get_cdclk_freq(struct device *dev)
> > return ret;
> > }
> >
> > +static int i915_audio_component_set_ncts(struct device *dev, int
> port,
> > + int dev_entry, 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 i;
> > + int n_low, n_up, n = 0;
>
> I think you'll need the power well get here, and put at the end. Or
> check that we it.
As we only need set the n/cts when playing the audio,
this can make sure that audio driver has already required
the power before we call the callback function.
Regards,
Libin
>
> > +
> > + /* 1. get the pipe */
> > + for_each_intel_encoder(drm_dev, intel_encoder) {
> > + 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 == -1) {
> > + 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. Need set the N/CTS manually at some frequencies */
> > + if ((mode->clock != TMDS_297M) &&
> > + (mode->clock != TMDS_296M)) {
> > + tmp = I915_READ(HSW_AUD_CFG(pipe));
> > + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > + I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > + return 0;
> > + }
> > +
> > +
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
Hi Jani,
Thanks for careful reviewing for all the patches, please
see my comments.
> -Original Message-
> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> Sent: Monday, August 10, 2015 8:10 PM
> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in
> modeset
>
> On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> > From: Libin Yang
> >
> > When modeset occurs and the TMDS frequency is set to some
> > speical value, the N/CTS need to be set manually if audio
> > is playing.
>
> When modeset occurs, we disable everything, and then re-enable
> everything, and notify the audio driver every step of the way. IIUC
> there should be no audio playing across the modeset, and when the
> modeset is complete and we've set the valid ELD, the audio driver
> should
> call the callback from the earlier patches to set the correct audio
> rate.
>
> Why is this patch needed? Please enlighten me.
Please image this scenario: when audio is playing, the gfx driver
does the modeset. In this situation, after modeset, audio driver
will do nothing and continue playing. And audio driver will not call
set_ncts.
>
> Also some comments in-line, provided you've convinced me first. ;)
>
> > Signed-off-by: Libin Yang
> > ---
> > drivers/gpu/drm/i915/i915_reg.h| 6 ++
> > drivers/gpu/drm/i915/intel_audio.c | 42
> ++
> > 2 files changed, 48 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > index da2d128..85f3beb 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7030,6 +7030,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 eddf37f..082f96d 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -235,6 +235,9 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> > const uint8_t *eld = connector->eld;
> > uint32_t tmp;
> > int len, i;
> > + int cvt_idx;
> > + int n_low, n_up, n;
> > + int base_rate, mul, div, rate;
> >
> > DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes
> ELD\n",
> > pipe_name(pipe), drm_eld_size(eld));
> > @@ -267,6 +270,21 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> > tmp |= AUDIO_ELD_VALID(pipe);
> > I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> >
> > + if ((mode->clock == 297000) ||
> > + (mode->clock == DIV_ROUND_UP(297000 * 1000,
> 1001))) {
> > + 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;
> > + }
>
> This should be abstracted to a separate function.
Yes. I will do it.
>
> > +
> > /* Enable timestamps */
> > tmp = I915_READ(HSW_AUD_CFG(pipe));
> > tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > @@ -276,6 +294,30 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> > tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > else
> > tmp |= audio_config_hdmi_pixel_clock(mode);
> > +
> > + if ((mode->clock != 297000) &&
> > + (mode->clock != DIV_ROUND_UP(297000 * 1000,
> 1001))) {
> > + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > + } else {
> > + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> > + if ((rate == aud_ncts[i].sample_rate) &&
> > + (mode->clock == aud_ncts[i].clock)) {
> > + n = aud_ncts[i].n;
> > + break;
> > + }
> > + }
> > + if (n != 0) {
> > + tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > + n_low = n & 0xfff;
> > + n_up = (n >> 12) & 0xff;
> > + tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > + tmp &= ~AUD_CO
Re: [Intel-gfx] [PATCH] drm/i915: Update HAS_PSR macro to include all gen>=8 platforms
I replied to Daniel last time. Pasting it on mailing list as well:
"Yes this is tested on android with HW tracking. Not sure about enabling by
default part. But this patch will be anyways required.
Regards,
Sonika"
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, July 21, 2015 3:18 PM
To: Lespiau, Damien
Cc: Jindal, Sonika; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Update HAS_PSR macro to include all
gen>=8 platforms
On Tue, Jul 21, 2015 at 10:31:19AM +0100, Damien Lespiau wrote:
> On Tue, Jul 21, 2015 at 02:48:31PM +0530, Sonika Jindal wrote:
> > This is to get PSR support for bxt.
> >
> > Signed-off-by: Sonika Jindal
>
> Maybe with a drm/i915/bxt prefix:
>
> Reviewed-by: Damien Lespiau
Is this actually tested? Can we maybe enable psr by default (Rodrigo seems so
close ...)?
-Daniel
>
> --
> Damien
>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h |5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index 718170c..54d2729 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2537,9 +2537,8 @@ struct drm_i915_cmd_table {
> >
> > #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
> > #define HAS_FPGA_DBG_UNCLAIMED(dev)(INTEL_INFO(dev)->has_fpga_dbg)
> > -#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)
> > || \
> > -IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
> > -IS_SKYLAKE(dev))
> > +#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_VALLEYVIEW(dev)
> > || \
> > +INTEL_INFO(dev)->gen >= 8)
> > #define HAS_RUNTIME_PM(dev)(IS_GEN6(dev) || IS_HASWELL(dev) || \
> > IS_BROADWELL(dev) || IS_VALLEYVIEW(dev) || \
> > IS_SKYLAKE(dev))
> > --
> > 1.7.10.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 02/21] drm/i915/gtt: Workaround for HW preload not flushing pdps
Hi Mika/Dave/Michel, I saw the patch of using LRI for root pointer update has been merged to drm-intel. When we consider i915 driver to run inside a virtual machine, e.g. with XenGT, we may still need Mika's this patch like below: " if (intel_vgpu_active(ppgtt->base.dev)) gen8_preallocate_top_level_pdps(ppgtt); " Could you share with us your opinion? Thanks in advance! The reason behind is that LRI command will make shadow PPGTT implementation hard. In XenGT, we construct shadow page table for each PPGTT in guest i915 driver, and then track every guest page table change in order to update shadow page table accordingly. The problem of page table updates with GPU command is that they cannot be trapped by hypervisor to finish the shadow page table update work. In XenGT, the only change we have is the command scan in context submission. But that is not exactly the right time to do shadow page table update. Mika's patch can address the problem nicely. With the preallocation, the root pointers in EXECLIST context will always keep the same. Then we can treat any attempt to change guest PPGTT with GPU commands as malicious behavior. Thanks! Regards, -Zhiyuan On Thu, Jun 11, 2015 at 04:57:42PM +0300, Mika Kuoppala wrote: > Dave Gordon writes: > > > On 10/06/15 12:42, Michel Thierry wrote: > >> On 5/29/2015 1:53 PM, Michel Thierry wrote: > >>> On 5/29/2015 12:05 PM, Michel Thierry wrote: > On 5/22/2015 6:04 PM, Mika Kuoppala wrote: > > With BDW/SKL and 32bit addressing mode only, the hardware preloads > > pdps. However the TLB invalidation only has effect on levels below > > the pdps. This means that if pdps change, hw might access with > > stale pdp entry. > > > > To combat this problem, preallocate the top pdps so that hw sees > > them as immutable for each context. > > > > Cc: Ville Syrjälä > > Cc: Rafael Barbalho > > Signed-off-by: Mika Kuoppala > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 50 > > + > > drivers/gpu/drm/i915/i915_reg.h | 17 + > > drivers/gpu/drm/i915/intel_lrc.c| 15 +-- > > 3 files changed, 68 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > > b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 0ffd459..1a5ad4c 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -941,6 +941,48 @@ err_out: > > return ret; > > } > > > > +/* With some architectures and 32bit legacy mode, hardware pre-loads > > the > > + * top level pdps but the tlb invalidation only invalidates the > > lower levels. > > + * This might lead to hw fetching with stale pdp entries if top level > > + * structure changes, ie va space grows with dynamic page tables. > > + */ > > > > Is this still necessary if we reload PDPs via LRI instructions whenever > > the address map has changed? That always (AFAICT) causes sufficient > > invalidation, so then we might not need to preallocate at all :) > > > > LRI reload gets my vote. Please ignore this patch. > -Mika > > > .Dave. > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Enable querying offset of UV plane with intel_plane_obj_offset
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6929 -Summary- Platform Delta drm-intel-nightly Series Applied ILK -1 302/302 301/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT 283/283 283/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_flip@wf_vblank-vs-modeset-interruptible PASS(1) DMESG_WARN(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts callback
> -Original Message-
> From: Yang, Libin
> Sent: Tuesday, August 11, 2015 10:41 AM
> To: 'Jani Nikula'; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: RE: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts
> callback
>
> Hi Jani,
>
> Thanks for reviewing, please see my comments
>
> > -Original Message-
> > From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> > Sent: Monday, August 10, 2015 7:46 PM
> > To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> > g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> > Subject: Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts
> > callback
> >
> > On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> > > From: Libin Yang
> > >
> > > Add the set_ncts callback.
> > >
> > > With the callback, audio driver can trigger
> > > i915 driver to set the proper N/CTS
> > > based on different sample rates.
> > >
> > > Signed-off-by: Libin Yang
> > > ---
> > > include/drm/i915_component.h | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/include/drm/i915_component.h
> > b/include/drm/i915_component.h
> > > index c9a8b64..7305881 100644
> > > --- a/include/drm/i915_component.h
> > > +++ b/include/drm/i915_component.h
> > > @@ -33,6 +33,8 @@ struct i915_audio_component {
> > > void (*put_power)(struct device *);
> > > void (*codec_wake_override)(struct device *, bool
> > enable);
> > > int (*get_cdclk_freq)(struct device *);
> > > + int (*set_ncts)(struct device *, int port, int dev_entry,
> > > + int rate);
> >
> > I'd rather call this set_audio_rate or similar, and dropping the
> > references to N and CTS. The caller does not need to know.
>
> But it seems the set_audio_rate will confuse the user. From the
> literal meaning, it is setting the rate of audio, such as sample rate,
> which will make audio driver developers confused.
> And the function is just setting N/CTS which is defined from
> HDMI SPEC.
After a second thought, set_ncts is not very good, we should
consider DP's naming and handling DP mode together.
>
> BTW: your comment reminds me that get_cdclk_freq() seems
> to need change the name as cdclk is not in the open spec.
>
> >
> > I'm also not fond of adding a dev_entry parameter and leaving it
> > unused. I do not think we know specifically how we're going to
> identify
> > MST sinks, and the interface may need to be changed anyway. Let's
> > force
> > an update in the caller side as well when there's actually sensible
> > support in our side.
>
> The device entry is a concept in HDA spec. For driver implementation,
> I think we can use an int variable or a struct device to represent it.
> A int variable is an easy way. There is some place in audio driver using
> int variable to represent the deivce entry. And audio driver will not
> care how gfx driver supports the DP1.2 MST, I mean audio driver will
> not know the structures inside gfx driver.
>
> I will remove this parameter in this version if you are thinking the
> MST interface is indeterminate.
>
> BTW: do you know when gfx will normally support MST?
>
> Best Regards,
> Libin
>
> >
> > BR,
> > Jani.
> >
> > > } *ops;
> > > };
> > >
> > > --
> > > 1.9.1
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
