Re: [Intel-gfx] [PATCH] drm/i915:gen9: Add WA for disable gather at set shader bit
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_CHICKEN20x7014
> +#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.
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.
--
Ben Widawsky, 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/skl: Update DDI buffer translation programming.
On Wed, Aug 5, 2015 at 6:47 PM Zhang, Xiong Y
wrote:
> Hi, Vivi:
> Do you think this patch could resolve the following two issues ?
> https://bugs.freedesktop.org/show_bug.cgi?id=91050
> https://bugs.freedesktop.org/show_bug.cgi?id=91269
I'm not sure honestly... I was just following latest spec updates. But I
believe it has potential..
at least to avoid this vswig parameters on our SDPs.
>
>
> thanks
> > -Original Message-
> > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
> Behalf Of
> > Rodrigo Vivi
> > Sent: Thursday, August 6, 2015 5:59 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Runyan, Arthur J; Vivi, Rodrigo
> > Subject: [Intel-gfx] [PATCH] drm/i915/skl: Update DDI buffer translation
> > programming.
> >
> > SKL-Y can now use the same programming for all VccIO values after an
> > adjustment to I_boost.
> > SKL-U DP table adjustments.
> > 1. Remove SKL Y 0.95V from "SKL H and S" columns in all tables.
> The
> > other SKL Y column removes the "0.85V VccIO" so it now applies to all
> > voltages.
> > 2. DP table changes SKL U 400mV+0db dword 0 value from 2016h to
> > 201Bh.
> > 3. DP table changes SKL U 600mv+0db dword 0 value from 2016h to
> > 201Bh.
> > 4. DP table increases I_boost to level 3 for SKL Y 400mv+9.5db.
> >
> > Reference: Graphics Spec Change r97962
> > Cc: Arthur Runyan
> > Signed-off-by: Rodrigo Vivi
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 73
> ++--
> > 1 file changed, 25 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 9a40bfb..9e5a21b 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -128,7 +128,7 @@ static const struct ddi_buf_trans
> > bdw_ddi_translations_hdmi[] = {
> > { 0x80FF, 0x001B0002, 0x0 },/* 9: 100010000 */
> > };
> >
> > -/* Skylake H, S, and Skylake Y with 0.95V VccIO */
> > +/* Skylake H and S */
> > static const struct ddi_buf_trans skl_ddi_translations_dp[] = {
> > { 0x2016, 0x00A0, 0x0 },
> > { 0x5012, 0x009B, 0x0 },
> > @@ -143,23 +143,23 @@ static const struct ddi_buf_trans
> > skl_ddi_translations_dp[] = {
> >
> > /* Skylake U */
> > static const struct ddi_buf_trans skl_u_ddi_translations_dp[] = {
> > - { 0x2016, 0x00A2, 0x0 },
> > + { 0x201B, 0x00A2, 0x0 },
> > { 0x5012, 0x0088, 0x0 },
> > { 0x7011, 0x0087, 0x0 },
> > - { 0x80009010, 0x00C7, 0x1 },/* Uses I_boost */
> > - { 0x2016, 0x009D, 0x0 },
> > + { 0x80009010, 0x00C7, 0x1 },/* Uses I_boost level 0x1
> */
> > + { 0x201B, 0x009D, 0x0 },
> > { 0x5012, 0x00C7, 0x0 },
> > { 0x7011, 0x00C7, 0x0 },
> > { 0x2016, 0x0088, 0x0 },
> > { 0x5012, 0x00C7, 0x0 },
> > };
> >
> > -/* Skylake Y with 0.85V VccIO */
> > -static const struct ddi_buf_trans skl_y_085v_ddi_translations_dp[] = {
> > +/* Skylake Y */
> > +static const struct ddi_buf_trans skl_y_ddi_translations_dp[] = {
> > { 0x0018, 0x00A2, 0x0 },
> > { 0x5012, 0x0088, 0x0 },
> > { 0x7011, 0x0087, 0x0 },
> > - { 0x80009010, 0x00C7, 0x1 },/* Uses I_boost */
> > + { 0x80009010, 0x00C7, 0x3 },/* Uses I_boost level 0x3
> */
> > { 0x0018, 0x009D, 0x0 },
> > { 0x5012, 0x00C7, 0x0 },
> > { 0x7011, 0x00C7, 0x0 },
> > @@ -168,7 +168,7 @@ static const struct ddi_buf_trans
> > skl_y_085v_ddi_translations_dp[] = { };
> >
> > /*
> > - * Skylake H and S, and Skylake Y with 0.95V VccIO
> > + * Skylake H and S
> > * eDP 1.4 low vswing translation parameters
> > */
> > static const struct ddi_buf_trans skl_ddi_translations_edp[] = { @@
> -202,10
> > +202,10 @@ static const struct ddi_buf_trans
> skl_u_ddi_translations_edp[] =
> > { };
> >
> > /*
> > - * Skylake Y with 0.95V VccIO
> > + * Skylake Y
> > * eDP 1.4 low vswing translation parameters
> > */
> > -static const struct ddi_buf_trans skl_y_085v_ddi_translations_edp[] = {
> > +static const struct ddi_buf_trans skl_y_ddi_translations_edp[] = {
> > { 0x0018, 0x00A8, 0x0 },
> > { 0x4013, 0x00AB, 0x0 },
> > { 0x7011, 0x00A4, 0x0 },
> > @@ -218,7 +218,7 @@ static const struct ddi_buf_trans
> > skl_y_085v_ddi_translations_edp[] = {
> > { 0x0018, 0x008A, 0x0 },
> > };
> >
> > -/* Skylake H, S and U, and Skylake Y with 0.95V VccIO */
> > +/* Skylake U, H and S */
> > static const struct ddi_buf_trans skl_ddi_translations_hdmi[] = {
> > { 0x0018, 0x00AC, 0x0 },
> > { 0x5012, 0x009D, 0x0 },
> > @@ -233,8 +233,8 @@ static const struct ddi_buf_trans
> > skl_ddi_translations_hdmi[] = {
> > { 0x0018, 0x00C7, 0x0 },
> > };
> >
> > -/* Skylake Y with 0.
[Intel-gfx] [PATCH 8/6] drm/i915/skl: Enable DDI-E
There are OEMs using DDI-E out there,
so let's enable it.
Unfortunately there is no detection bit for DDI-E
So we need to rely on VBT for that.
I also need to give credits to Xiong since before seing
his approach to check info->support_* I was creating an ugly
vbt->ddie_sfuse_strap in order to propagate the ddi presence info
v2: Rebased as last patch in the series. since all other patches
in this series are needed for anything working propperly on DDI-E.
Credits-to: "Zhang, Xiong Y"
Cc: "Zhang, Xiong Y"
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_bios.c| 14 +++---
drivers/gpu/drm/i915/intel_bios.h| 2 ++
drivers/gpu/drm/i915/intel_display.c | 9 +
3 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index c417973..c5b5a59b 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -898,19 +898,19 @@ static void parse_ddi_port(struct drm_i915_private
*dev_priv, enum port port,
/* Each DDI port can have more than one value on the "DVO Port" field,
* so look for all the possible values for each port and abort if more
* than one is found. */
- int dvo_ports[][2] = {
- {DVO_PORT_HDMIA, DVO_PORT_DPA},
- {DVO_PORT_HDMIB, DVO_PORT_DPB},
- {DVO_PORT_HDMIC, DVO_PORT_DPC},
- {DVO_PORT_HDMID, DVO_PORT_DPD},
- {DVO_PORT_CRT, -1 /* Port E can only be DVO_PORT_CRT */ },
+ int dvo_ports[][3] = {
+ {DVO_PORT_HDMIA, DVO_PORT_DPA, -1},
+ {DVO_PORT_HDMIB, DVO_PORT_DPB, -1},
+ {DVO_PORT_HDMIC, DVO_PORT_DPC, -1},
+ {DVO_PORT_HDMID, DVO_PORT_DPD, -1},
+ {DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE},
};
/* Find the child device to use, abort if more than one found. */
for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
it = dev_priv->vbt.child_dev + i;
- for (j = 0; j < 2; j++) {
+ for (j = 0; j < 3; j++) {
if (dvo_ports[port][j] == -1)
break;
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index 77bf1dd..a2ef0df 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -759,6 +759,8 @@ int intel_parse_bios(struct drm_device *dev);
#define DVO_PORT_DPC 8
#define DVO_PORT_DPD 9
#define DVO_PORT_DPA 10
+#define DVO_PORT_DPE 11
+#define DVO_PORT_HDMIE 12
#define DVO_PORT_MIPIA 21
#define DVO_PORT_MIPIB 22
#define DVO_PORT_MIPIC 23
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index fcf1230..7bf6209 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13961,6 +13961,15 @@ static void intel_setup_outputs(struct drm_device *dev)
intel_ddi_init(dev, PORT_C);
if (found & SFUSE_STRAP_DDID_DETECTED)
intel_ddi_init(dev, PORT_D);
+ /*
+* On SKL we don't have a way to detect DDI-E so we rely on VBT.
+*/
+ if (IS_SKYLAKE(dev) &&
+ (dev_priv->vbt.ddi_port_info[PORT_E].supports_dp ||
+dev_priv->vbt.ddi_port_info[PORT_E].supports_dvi ||
+dev_priv->vbt.ddi_port_info[PORT_E].supports_hdmi))
+ intel_ddi_init(dev, PORT_E);
+
} else if (HAS_PCH_SPLIT(dev)) {
int found;
dpd_is_edp = intel_dp_is_edp(dev, PORT_D);
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes.
DDI-A and DDI-E shares the 4 lanes. So when DDI-E is present we need to configure lane count propperly for both. This was based on Sonika's [PATCH] drm/i915/skl: Select DDIA lane capability based upon vbt Credits-to: Sonika Jindal Cc: Xiong Zhang Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_ddi.c | 12 ++-- drivers/gpu/drm/i915/intel_dp.c | 8 +--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 110d546..557cecf 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -3178,7 +3178,15 @@ void intel_ddi_init(struct drm_device *dev, enum port port) struct intel_digital_port *intel_dig_port; struct intel_encoder *intel_encoder; struct drm_encoder *encoder; - bool init_hdmi, init_dp; + bool init_hdmi, init_dp, ddi_e_present; + + /* +* On SKL we don't have a way to detect DDI-E so we rely on VBT. +*/ + ddie_present = IS_SKYLAKE(dev) && + (dev_priv->vbt.ddi_port_info[PORT_E].supports_dp || +dev_priv->vbt.ddi_port_info[PORT_E].supports_dvi || +dev_priv->vbt.ddi_port_info[PORT_E].supports_hdmi); init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || dev_priv->vbt.ddi_port_info[port].supports_hdmi); @@ -3210,7 +3218,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port) intel_dig_port->port = port; intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & (DDI_BUF_PORT_REVERSAL | - DDI_A_4_LANES); + ddi_e_present ? 0 : DDI_A_4_LANES); intel_encoder->type = INTEL_OUTPUT_UNKNOWN; intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3ff2080..7ada79e 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -159,9 +159,11 @@ static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp) u8 source_max, sink_max; source_max = 4; - if (HAS_DDI(dev) && intel_dig_port->port == PORT_A && - (intel_dig_port->saved_port_bits & DDI_A_4_LANES) == 0) - source_max = 2; + if (HAS_DDI(dev) && (intel_dig_port->port == PORT_E || +(intel_dig_port->port == PORT_A && + (intel_dig_port->saved_port_bits & + DDI_A_4_LANES) == 0)) + source_max = 2; sink_max = drm_dp_max_lane_count(intel_dp->dpcd); -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Enable HDMI on DDI-E
On Thu, Aug 6, 2015 at 12:45 AM Xiong Zhang wrote:
> DDI-E doesn't have the correspondent GMBUS pin.
>
> We rely on VBT to tell us which one it being used instead.
>
> The DVI/HDMI on shared port couldn't exist.
>
> This patch isn't tested without hardware wchich has HDMI
> on DDI-E.
>
> Signed-off-by: Xiong Zhang
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 +
> drivers/gpu/drm/i915/intel_bios.c | 28
> drivers/gpu/drm/i915/intel_hdmi.c | 18 ++
> 3 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 6d93334..5d8e7fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1414,6 +1414,10 @@ enum modeset_restore {
> #define DP_AUX_C 0x20
> #define DP_AUX_D 0x30
>
> +#define DDC_PIN_B 0x05
> +#define DDC_PIN_C 0x04
> +#define DDC_PIN_D 0x06
> +
> struct ddi_vbt_port_info {
> /*
> * This is an index in the HDMI/DVI DDI buffer translation table.
> @@ -1428,6 +1432,7 @@ struct ddi_vbt_port_info {
> uint8_t supports_dp:1;
>
> uint8_t alternate_aux_channel;
> + uint8_t alternate_ddc_pin;
> };
>
> enum psr_lines_to_wait {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 16cdf17..265227f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -894,7 +894,7 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> uint8_t hdmi_level_shift;
> int i, j;
> bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> - uint8_t aux_channel;
> + uint8_t aux_channel, ddc_pin;
> /* Each DDI port can have more than one value on the "DVO Port"
> field,
> * so look for all the possible values for each port and abort if
> more
> * than one is found. */
> @@ -928,6 +928,7 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> return;
>
> aux_channel = child->raw[25];
> + ddc_pin = child->common.ddc_pin;
>
> is_dvi = child->common.device_type &
> DEVICE_TYPE_TMDS_DVI_SIGNALING;
> is_dp = child->common.device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT;
> @@ -959,11 +960,30 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
>
> if (is_dvi) {
> - if (child->common.ddc_pin == 0x05 && port != PORT_B)
> + if (port == PORT_E) {
> + info->alternate_ddc_pin = ddc_pin;
> + /* if DDIE share ddc pin with other port, then
> +* dvi/hdmi couldn't exist on the shared port.
>
I see a trailing space here
+* Otherwise they share the same ddc bin and system
> +* couldn't communicate with them seperately. */
> + if (ddc_pin == DDC_PIN_B) {
> +
> dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
> +
> dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
> + }
> + else if (ddc_pin == DDC_PIN_C) {
> +
> dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
> +
> dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
> + }
>
and here
But the concept and patch is totally right so with trailing whitespaces
fixed
Reviewed-by: Rodrigo Vivi
> + else if (ddc_pin == DDC_PIN_D) {
> +
> dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
> +
> dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
> + }
> + }
> + else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> - if (child->common.ddc_pin == 0x04 && port != PORT_C)
> + else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> - if (child->common.ddc_pin == 0x06 && port != PORT_D)
> + else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> }
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..e1f6e81 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1991,6 +1991,24 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
> intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> intel_encoder->hpd_pin = HPD_PORT_D;
> break;
> + case PORT_E:
> + /* On SKL PORT E doesn't have seperate GMBUS pin
> +* We rely on VBT to set a proper alternate GMBUS pin. */
> +
Re: [Intel-gfx] [PATCH 5/6] drm/i915/skl: enable DDIE hotplug
Great!
Reviewed-by: Rodrigo Vivi
On Thu, Aug 6, 2015 at 12:45 AM Xiong Zhang wrote:
> Signed-off-by: Xiong Zhang
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_irq.c | 49
> +---
> drivers/gpu/drm/i915/i915_reg.h | 12 +
> drivers/gpu/drm/i915/intel_display.c | 2 ++
> drivers/gpu/drm/i915/intel_dp.c | 3 +++
> drivers/gpu/drm/i915/intel_hotplug.c | 3 +++
> 6 files changed, 66 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 8d50d4f..6d93334 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -214,6 +214,7 @@ enum hpd_pin {
> HPD_PORT_B,
> HPD_PORT_C,
> HPD_PORT_D,
> + HPD_PORT_E,
> HPD_NUM_PINS
> };
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 1118c39..420cf5a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -61,6 +61,13 @@ static const u32 hpd_cpt[HPD_NUM_PINS] = {
> [HPD_PORT_D] = SDE_PORTD_HOTPLUG_CPT
> };
>
> +static const u32 hpd_spt[HPD_NUM_PINS] = {
> + [HPD_PORT_B] = SDE_PORTB_HOTPLUG_CPT,
> + [HPD_PORT_C] = SDE_PORTC_HOTPLUG_CPT,
> + [HPD_PORT_D] = SDE_PORTD_HOTPLUG_CPT,
> + [HPD_PORT_E] = SDE_PORTE_HOTPLUG_SPT
> +};
> +
> static const u32 hpd_mask_i915[HPD_NUM_PINS] = {
> [HPD_CRT] = CRT_HOTPLUG_INT_EN,
> [HPD_SDVO_B] = SDVOB_HOTPLUG_INT_EN,
> @@ -1252,6 +1259,8 @@ static bool pch_port_hotplug_long_detect(enum port
> port, u32 val)
> return val & PORTC_HOTPLUG_LONG_DETECT;
> case PORT_D:
> return val & PORTD_HOTPLUG_LONG_DETECT;
> + case PORT_E:
> + return val & PORTE_HOTPLUG_LONG_DETECT;
> default:
> return false;
> }
> @@ -1752,7 +1761,12 @@ static void cpt_irq_handler(struct drm_device *dev,
> u32 pch_iir)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> int pipe;
> - u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
> + u32 hotplug_trigger;
> +
> + if (HAS_PCH_SPT(dev))
> + hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT;
> + else
> + hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
>
> if (hotplug_trigger) {
> u32 dig_hotplug_reg, pin_mask, long_mask;
> @@ -1760,9 +1774,24 @@ static void cpt_irq_handler(struct drm_device *dev,
> u32 pch_iir)
> dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>
> - intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> - dig_hotplug_reg, hpd_cpt,
> - pch_port_hotplug_long_detect);
> + if (HAS_PCH_SPT(dev)) {
> + intel_get_hpd_pins(&pin_mask, &long_mask,
> + hotplug_trigger,
> + dig_hotplug_reg, hpd_spt,
> + pch_port_hotplug_long_detect);
> +
> + /* detect PORTE HP event */
> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
> + if (pch_port_hotplug_long_detect(PORT_E,
> +dig_hotplug_reg))
> + long_mask |= 1 << HPD_PORT_E;
> + }
> + else
> + intel_get_hpd_pins(&pin_mask, &long_mask,
> + hotplug_trigger,
> + dig_hotplug_reg, hpd_cpt,
> + pch_port_hotplug_long_detect);
> +
> intel_hpd_irq_handler(dev, pin_mask, long_mask);
> }
>
> @@ -2984,6 +3013,11 @@ static void ibx_hpd_irq_setup(struct drm_device
> *dev)
> for_each_intel_encoder(dev, intel_encoder)
> if
> (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> enabled_irqs |=
> hpd_ibx[intel_encoder->hpd_pin];
> + } else if (HAS_PCH_SPT(dev)) {
> + hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
> + for_each_intel_encoder(dev, intel_encoder)
> + if
> (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> + enabled_irqs |=
> hpd_spt[intel_encoder->hpd_pin];
> } else {
> hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
> for_each_intel_encoder(dev, intel_encoder)
> @@ -3005,6 +3039,13 @@ static void ibx_hpd_irq_setup(struct drm_device
> *dev)
> hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
> hotplug |= PORTB_HOTPLUG_ENABLE | P
[Intel-gfx] [PATCH] drm/i915: Set alternate aux for DDI-E
There is no correspondent Aux channel for DDI-E.
So we need to rely on VBT to let us know witch one
is being used instead.
v2: Removing some trailing spaces and giving proper
credit to Xiong that added a nice way to avoid port
conflicts by setting supports_dp = 0 when using
equivalent aux for DDI-E.
Credits-to: Xiong Zhang
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/i915_drv.h | 7 +++
drivers/gpu/drm/i915/intel_bios.c | 23 +++
drivers/gpu/drm/i915/intel_ddi.c | 5 ++---
drivers/gpu/drm/i915/intel_dp.c | 29 -
4 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4932d29..3ffd962 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1409,6 +1409,11 @@ enum modeset_restore {
MODESET_SUSPENDED,
};
+#define DP_AUX_A 0x40
+#define DP_AUX_B 0x10
+#define DP_AUX_C 0x20
+#define DP_AUX_D 0x30
+
struct ddi_vbt_port_info {
/*
* This is an index in the HDMI/DVI DDI buffer translation table.
@@ -1421,6 +1426,8 @@ struct ddi_vbt_port_info {
uint8_t supports_dvi:1;
uint8_t supports_hdmi:1;
uint8_t supports_dp:1;
+
+ uint8_t alternate_aux_channel;
};
enum psr_lines_to_wait {
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 31b1079..990acc2 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -968,13 +968,28 @@ static void parse_ddi_port(struct drm_i915_private
*dev_priv, enum port port,
}
if (is_dp) {
- if (aux_channel == 0x40 && port != PORT_A)
+ if (port == PORT_E) {
+ info->alternate_aux_channel = aux_channel;
+ /* if DDIE share aux channel with other port, then
+* DP couldn't exist on the shared port. Otherwise
+* they share the same aux channel and system
+* couldn't communicate with them seperately. */
+ if (aux_channel == DP_AUX_A)
+ dev_priv->vbt.ddi_port_info[PORT_A].supports_dp
= 0;
+ else if (aux_channel == DP_AUX_B)
+ dev_priv->vbt.ddi_port_info[PORT_B].supports_dp
= 0;
+ else if (aux_channel == DP_AUX_C)
+ dev_priv->vbt.ddi_port_info[PORT_C].supports_dp
= 0;
+ else if (aux_channel == DP_AUX_D)
+ dev_priv->vbt.ddi_port_info[PORT_D].supports_dp
= 0;
+ }
+ else if (aux_channel == DP_AUX_A && port != PORT_A)
DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
- if (aux_channel == 0x10 && port != PORT_B)
+ else if (aux_channel == DP_AUX_B && port != PORT_B)
DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
- if (aux_channel == 0x20 && port != PORT_C)
+ else if (aux_channel == DP_AUX_C && port != PORT_C)
DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
- if (aux_channel == 0x30 && port != PORT_D)
+ else if (aux_channel == DP_AUX_D && port != PORT_D)
DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
}
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9a40bfb..110d546 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3184,10 +3184,9 @@ void intel_ddi_init(struct drm_device *dev, enum port
port)
dev_priv->vbt.ddi_port_info[port].supports_hdmi);
init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
if (!init_dp && !init_hdmi) {
- DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible,
assuming it is\n",
+ DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible,
respect it\n",
port_name(port));
- init_hdmi = true;
- init_dp = true;
+ return;
}
intel_dig_port = kzalloc(sizeof(*intel_dig_port), GFP_KERNEL);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 15f0d72..601a12a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1033,11 +1033,34 @@ static void
intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
enum port port = intel_dig_port->port;
+ struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
const char *name = NUL
Re: [Intel-gfx] [PATCH mesa v3] i965/gen8+: bo in state base address must be in 32-bit address range
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)
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.
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/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -409,11 +409,23 @@ bool
> intel_batchbuffer_emit_reloc64(struct brw_context *brw,
> drm_intel_bo *buffer,
> uint
Re: [Intel-gfx] [Mesa-dev] [PATCH mesa v3] i965/gen8+: bo in state base address must be in 32-bit address range
On 7 August 2015 at 20:57, Ilia Mirkin wrote: > On Fri, Aug 7, 2015 at 5: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. >> >> 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 > > There is no such version. I think you need a release before you can > commit this. Otherwise you'll cause pain for a whole lot of people. > Afaics Michel explicitly covered the way things will/should be merged in the cover letter. -Emil ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-fixes
Hi Linus, One i915 regression fix and a drm core one since Dave's not around, both introduced in 4.2 so not cc: stable. The fix for the warning Ted reported isn't in here yet since he didn't yet supply a tested-by and I can't repro this one myself (it's in fixup code that needs firmware doing something i915 wouldn't do). Cheers, Daniel The following changes since commit 5eb3e5a5e11d14f9deb2a4b83555443b69ab9940: drm/i915: Declare the swizzling unknown for L-shaped configurations (2015-07-30 16:51:20 +0200) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2015-08-07 for you to fetch changes up to 209e4dbc8dcdb2b1839f18fd1cf07ec7bedadf4d: drm/vblank: Use u32 consistently for vblank counters (2015-08-07 14:35:53 +0200) Daniel Vetter (1): drm/vblank: Use u32 consistently for vblank counters David Weinehall (1): drm/i915: Allow parsing of variable size child device entries from VBT drivers/gpu/drm/drm_irq.c | 2 +- drivers/gpu/drm/i915/intel_bios.c | 27 +++ include/drm/drmP.h| 2 +- 3 files changed, 25 insertions(+), 6 deletions(-) -- 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 2/4] drm/i915: Use complete virtual address range on 32-bit platforms
On Fri, Aug 07, 2015 at 05:40:18PM +0100, Michel Thierry wrote:
> With the offset length being taken care of in ("drm/i915/gtt: Allow >=
> 4GB offsets in X86_32"), the code should be finally safe in 32-bit
> kernels.
>
> This reverts commit 501fd70fcaebc911b6b96a7b331e6960e5af67e7
> Author: Michel Thierry
> Date: Fri May 29 14:15:05 2015 +0100
>
> drm/i915: limit PPGTT size to 2GB in 32-bit platforms
>
> Signed-off-by: Michel Thierry
I think we should be safe
Reviewed-by: Chris Wilson
The biggest problem is that who do have testing 32bit gen8+?
-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 3/4] drm/i915: Always pass dev pointer in pdp_init
On Fri, Aug 07, 2015 at 05:40:19PM +0100, Michel Thierry wrote: > And fix 0-DAY kernel test infrastructure warning. > > Reported-by: Fengguang Wu > Signed-off-by: Michel Thierry Nah, for extra credit, pass NULL! Not that this needs a r-b, 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/gtt: Allow >= 4GB offsets in X86_32
On Fri, Aug 07, 2015 at 05:40:17PM +0100, Michel Thierry wrote:
> Similar to commit c44ef60e437019b8ca1dab8b4d2e8761fd4ce1e9 ("drm/i915/gtt:
> Allow >= 4GB sizes for vm"), i915_gem_obj_offset and i915_gem_obj_ggtt_offset
> return an unsigned long, which in only 4-bytes long in 32-bit kernels.
>
> Change return type (and other related offset variables) to u64.
>
> Since Global GTT is always limited to 4GB, this change would not be required
> in i915_gem_obj_ggtt_offset, but this is done for consistency.
>
> v2: Remove unnecessary offset variable in do_pin, as we already have
> vma->node.start (Chris).
> Update GGTT offset too (Tvrtko).
>
> Cc: Chris Wilson
> Cc: Daniel Vetter
> Cc: Tvrtko Ursulin
> Signed-off-by: Michel Thierry
There are a few more places where we use ggtt_offset() and ggtt_bound()
when we already have the vma, but they are only in the context of this
patch and not the meat. They just make me cringe.
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 mesa v3] i965/gen8+: bo in state base address must be in 32-bit address range
On Fri, Aug 7, 2015 at 5: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.
>
> 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
There is no such version. I think you need a release before you can
commit this. Otherwise you'll cause pain for a whole lot of people.
> 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/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -409,11 +409,23 @@ bool
> intel_batchbuffer_emit_reloc64(struct brw_context *brw,
> drm_intel_bo *buffer,
> uint32_t read_domains, uint32_t write_domain,
> - uint32_t delta)
> + uint32_t delta,
> + bool support_48bit_offset)
> {
> - int ret = drm_intel_bo_emit_reloc(brw->batch.bo, 4*brw->batch.used,
> - buffer, delta,
> - read_domains, write_domain);
> + int ret;
> +
> + /* Not all buffers can be allocated outside the first 4GB, and
> +* offset must be limited to 32-bits.
> +*/
> + if (support_48bit_offset)
> + drm_intel_bo_emit_reloc_48bit(brw->batch.bo, 4*brw->batch.
[Intel-gfx] [PATCH] drm/i915: Report IOMMU enabled status for GPU hangs
The IOMMU for Intel graphics has historically had many issues resulting
in random GPU hangs. Lets include its status when capturing the GPU hang
error state for post-mortem analysis.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gpu_error.c | 5 +
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 786ddd8b4e97..82d225786db2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -488,6 +488,7 @@ struct drm_i915_error_state {
struct timeval time;
char error_msg[128];
+ int iommu;
u32 reset_count;
u32 suspend_count;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0110c2626859..21ae99e5b1ed 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -399,6 +399,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf
*m,
err_printf(m, "Reset count: %u\n", error->reset_count);
err_printf(m, "Suspend count: %u\n", error->suspend_count);
err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
+ err_printf(m, "IOMMU enabled?: %d\n", error->iommu);
err_printf(m, "EIR: 0x%08x\n", error->eir);
err_printf(m, "IER: 0x%08x\n", error->ier);
if (INTEL_INFO(dev)->gen >= 8) {
@@ -1341,6 +1342,10 @@ static void i915_error_capture_msg(struct drm_device
*dev,
static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
struct drm_i915_error_state *error)
{
+ error->iommu = -1;
+#ifdef CONFIG_INTEL_IOMMU
+ error->iommu = intel_iommu_gfx_mapped;
+#endif
error->reset_count = i915_reset_count(&dev_priv->gpu_error);
error->suspend_count = dev_priv->suspend_count;
}
--
2.5.0
___
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 Fri, Aug 7, 2015 at 2:45 AM, Michel Thierry wrote: > 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/intel_batchbuffer.c > +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c > @@ -409,11 +409,23 @@ bool > intel_batchbuffer_emit_reloc64(struct brw_context *brw, This patch needs to be rebased on commit 09348c12f (committed more than 3 weeks ago). ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/core: Add optional reason for timeout failure
On 08/07/2015 10:29 AM, Daniel Vetter wrote:
> "Timed out" isn't a terribly informative message, allow users to set
> something more informative. Inspired by a request from Jesse.
>
> Cc: Jesse Barnes
> Signed-off-by: Daniel Vetter
> ---
> lib/igt_core.c | 12 ++--
> lib/igt_core.h | 3 ++-
> lib/igt_debugfs.c | 4 ++--
> lib/tests/igt_timeout.c | 2 +-
> tests/kms_flip.c| 4 ++--
> 5 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index af3d87316857..e2c2502bd147 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1748,9 +1748,13 @@ out:
> free(line);
> }
>
> +static const char *timeout_op;
> static void igt_alarm_handler(int signal)
> {
> - igt_info("Timed out\n");
> + if (timeout_op)
> + igt_info("Timed out: %s\n", timeout_op);
> + else
> + igt_info("Timed out\n");
>
> /* exit with failure status */
> igt_fail(IGT_EXIT_FAILURE);
> @@ -1759,6 +1763,7 @@ static void igt_alarm_handler(int signal)
> /**
> * igt_set_timeout:
> * @seconds: number of seconds before timeout
> + * @op: Optional string to explain what operation has timed out in the debug
> log
> *
> * Fail a test and exit with #IGT_EXIT_FAILURE status after the specified
> * number of seconds have elapsed. If the current test has subtests and the
> @@ -1768,7 +1773,8 @@ static void igt_alarm_handler(int signal)
> * Any previous timer is cancelled and no timeout is scheduled if @seconds is
> * zero.
> */
> -void igt_set_timeout(unsigned int seconds)
> +void igt_set_timeout(unsigned int seconds,
> + const char *op)
> {
> struct sigaction sa;
>
> @@ -1776,6 +1782,8 @@ void igt_set_timeout(unsigned int seconds)
> sigemptyset(&sa.sa_mask);
> sa.sa_flags = 0;
>
> + timeout_op = op;
> +
> if (seconds == 0)
> sigaction(SIGALRM, NULL, NULL);
> else
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index 83eac02b28bf..1a324ee85514 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -732,7 +732,8 @@ extern enum igt_log_level igt_log_level;
> } while (0)
>
>
> -void igt_set_timeout(unsigned int seconds);
> +void igt_set_timeout(unsigned int seconds,
> + const char *op);
>
> FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir,
> const char* filename);
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 568154ac0e80..6180a2aa56db 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -463,9 +463,9 @@ static bool read_one_crc(igt_pipe_crc_t *pipe_crc,
> igt_crc_t *out)
> ssize_t bytes_read;
> char buf[pipe_crc->buffer_len];
>
> - igt_set_timeout(5);
> + igt_set_timeout(5, "CRC reading");
> bytes_read = read(pipe_crc->crc_fd, &buf, pipe_crc->line_len);
> - igt_set_timeout(0);
> + igt_set_timeout(0, NULL);
>
> igt_assert_eq(bytes_read, pipe_crc->line_len);
> buf[bytes_read] = '\0';
> diff --git a/lib/tests/igt_timeout.c b/lib/tests/igt_timeout.c
> index 8affa61f3d79..d228041d493b 100644
> --- a/lib/tests/igt_timeout.c
> +++ b/lib/tests/igt_timeout.c
> @@ -3,6 +3,6 @@
>
> igt_simple_main
> {
> - igt_set_timeout(1);
> + igt_set_timeout(1, "Testcase");
> sleep(5);
> }
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 25c924305c32..214cd696fd2f 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -1614,9 +1614,9 @@ static void test_nonblocking_read(int in)
> }
> igt_require(ret != -1);
>
> - igt_set_timeout(5);
> + igt_set_timeout(5, "Nonblocking DRM fd reading");
> ret = read(fd, buffer, sizeof(buffer));
> - igt_set_timeout(0);
> + igt_set_timeout(0, NULL);
> igt_assert_eq(ret, -1);
> igt_assert_eq(errno, EAGAIN);
>
>
Reviewed-by: Jesse Barnes
The earlier version worked like a charm.
Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915:gen9: Add WA for disable gather at set shader bit
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_CHICKEN0x7018
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))) {
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/3] lib/core: Add igt_reset_timeout
Convenience wrapper suggested by Chris for igt_set_timeout(0, NULL).
v2: While at it add an empty line in kms_flip to make
set/reset_timeout a visual block.
Cc: Chris Wilson
Signed-off-by: Daniel Vetter
---
lib/igt_core.c| 14 +-
lib/igt_core.h| 2 ++
lib/igt_debugfs.c | 2 +-
tests/kms_flip.c | 3 ++-
4 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index e2c2502bd147..c70176fb4962 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1771,7 +1771,8 @@ static void igt_alarm_handler(int signal)
* marked as failed.
*
* Any previous timer is cancelled and no timeout is scheduled if @seconds is
- * zero.
+ * zero. But for clarity the timeout set with this function should be cleared
+ * with igt_reset_timeout().
*/
void igt_set_timeout(unsigned int seconds,
const char *op)
@@ -1792,6 +1793,17 @@ void igt_set_timeout(unsigned int seconds,
alarm(seconds);
}
+/**
+ * igt_reset_timeout - reset timeout to default
+ *
+ * This function resets a timeout set by igt_set_timeout() and disables any
+ * timer set up by the former function.
+ */
+void igt_reset_timeout(void)
+{
+ igt_set_timeout(0, NULL);
+}
+
FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir,
const char* filename)
{
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 1a324ee85514..1d77f4528dea 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -735,6 +735,8 @@ extern enum igt_log_level igt_log_level;
void igt_set_timeout(unsigned int seconds,
const char *op);
+void igt_reset_timeout(void);
+
FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir,
const char* filename);
/**
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 6180a2aa56db..3cbc981d684c 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -465,7 +465,7 @@ static bool read_one_crc(igt_pipe_crc_t *pipe_crc,
igt_crc_t *out)
igt_set_timeout(5, "CRC reading");
bytes_read = read(pipe_crc->crc_fd, &buf, pipe_crc->line_len);
- igt_set_timeout(0, NULL);
+ igt_reset_timeout();
igt_assert_eq(bytes_read, pipe_crc->line_len);
buf[bytes_read] = '\0';
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 214cd696fd2f..a595d9f1d69f 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1616,7 +1616,8 @@ static void test_nonblocking_read(int in)
igt_set_timeout(5, "Nonblocking DRM fd reading");
ret = read(fd, buffer, sizeof(buffer));
- igt_set_timeout(0, NULL);
+ igt_reset_timeout();
+
igt_assert_eq(ret, -1);
igt_assert_eq(errno, EAGAIN);
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/3] lib/core: Add optional reason for timeout failure
"Timed out" isn't a terribly informative message, allow users to set
something more informative. Inspired by a request from Jesse.
Cc: Jesse Barnes
Signed-off-by: Daniel Vetter
---
lib/igt_core.c | 12 ++--
lib/igt_core.h | 3 ++-
lib/igt_debugfs.c | 4 ++--
lib/tests/igt_timeout.c | 2 +-
tests/kms_flip.c| 4 ++--
5 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index af3d87316857..e2c2502bd147 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1748,9 +1748,13 @@ out:
free(line);
}
+static const char *timeout_op;
static void igt_alarm_handler(int signal)
{
- igt_info("Timed out\n");
+ if (timeout_op)
+ igt_info("Timed out: %s\n", timeout_op);
+ else
+ igt_info("Timed out\n");
/* exit with failure status */
igt_fail(IGT_EXIT_FAILURE);
@@ -1759,6 +1763,7 @@ static void igt_alarm_handler(int signal)
/**
* igt_set_timeout:
* @seconds: number of seconds before timeout
+ * @op: Optional string to explain what operation has timed out in the debug
log
*
* Fail a test and exit with #IGT_EXIT_FAILURE status after the specified
* number of seconds have elapsed. If the current test has subtests and the
@@ -1768,7 +1773,8 @@ static void igt_alarm_handler(int signal)
* Any previous timer is cancelled and no timeout is scheduled if @seconds is
* zero.
*/
-void igt_set_timeout(unsigned int seconds)
+void igt_set_timeout(unsigned int seconds,
+const char *op)
{
struct sigaction sa;
@@ -1776,6 +1782,8 @@ void igt_set_timeout(unsigned int seconds)
sigemptyset(&sa.sa_mask);
sa.sa_flags = 0;
+ timeout_op = op;
+
if (seconds == 0)
sigaction(SIGALRM, NULL, NULL);
else
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 83eac02b28bf..1a324ee85514 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -732,7 +732,8 @@ extern enum igt_log_level igt_log_level;
} while (0)
-void igt_set_timeout(unsigned int seconds);
+void igt_set_timeout(unsigned int seconds,
+const char *op);
FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir,
const char* filename);
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 568154ac0e80..6180a2aa56db 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -463,9 +463,9 @@ static bool read_one_crc(igt_pipe_crc_t *pipe_crc,
igt_crc_t *out)
ssize_t bytes_read;
char buf[pipe_crc->buffer_len];
- igt_set_timeout(5);
+ igt_set_timeout(5, "CRC reading");
bytes_read = read(pipe_crc->crc_fd, &buf, pipe_crc->line_len);
- igt_set_timeout(0);
+ igt_set_timeout(0, NULL);
igt_assert_eq(bytes_read, pipe_crc->line_len);
buf[bytes_read] = '\0';
diff --git a/lib/tests/igt_timeout.c b/lib/tests/igt_timeout.c
index 8affa61f3d79..d228041d493b 100644
--- a/lib/tests/igt_timeout.c
+++ b/lib/tests/igt_timeout.c
@@ -3,6 +3,6 @@
igt_simple_main
{
- igt_set_timeout(1);
+ igt_set_timeout(1, "Testcase");
sleep(5);
}
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 25c924305c32..214cd696fd2f 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1614,9 +1614,9 @@ static void test_nonblocking_read(int in)
}
igt_require(ret != -1);
- igt_set_timeout(5);
+ igt_set_timeout(5, "Nonblocking DRM fd reading");
ret = read(fd, buffer, sizeof(buffer));
- igt_set_timeout(0);
+ igt_set_timeout(0, NULL);
igt_assert_eq(ret, -1);
igt_assert_eq(errno, EAGAIN);
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/3] tests: Document ABI extension catchers
Our invalid-flags/params testcases are meant to catch abi extensions
by just testing for the next available flag/param. Unfortunately we
need that since without those we forgot to write testcases for these
new flags way too often :(
But it's not entirely clear why this is, so document this trick with
comments.
Also gem_wait wasn't this paranoid, so change the testcase to be so.
Signed-off-by: Daniel Vetter
---
tests/gem_ctx_param_basic.c | 3 +++
tests/gem_exec_params.c | 3 +++
tests/gem_wait.c| 5 -
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
index 1e7e8ff40703..430a53b22c57 100644
--- a/tests/gem_ctx_param_basic.c
+++ b/tests/gem_ctx_param_basic.c
@@ -149,6 +149,9 @@ igt_main
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
}
+ /* NOTE: This testcase intentionally tests for the next free parameter
+* to catch ABI extensions. Don't "fix" this testcase without adding all
+* the tests for the new param first. */
ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP + 1;
igt_subtest("invalid-param-get") {
diff --git a/tests/gem_exec_params.c b/tests/gem_exec_params.c
index e9c13a40f8a7..b33a7408c476 100644
--- a/tests/gem_exec_params.c
+++ b/tests/gem_exec_params.c
@@ -221,6 +221,9 @@ igt_main
/* HANDLE_LUT and NO_RELOC are already exercised by gem_exec_lut_handle
*/
igt_subtest("invalid-flag") {
+ /* NOTE: This test intentionally exercise the next available
+* flag. Don't "fix" this testcase without adding the required
+* tests for the new flag first. */
execbuf.flags = I915_EXEC_RENDER |
(LOCAL_I915_EXEC_RESOURCE_STREAMER << 1);
RUN_FAIL(EINVAL);
}
diff --git a/tests/gem_wait.c b/tests/gem_wait.c
index 958bf93ff5fd..a785b16e783e 100644
--- a/tests/gem_wait.c
+++ b/tests/gem_wait.c
@@ -236,7 +236,10 @@ static void invalid_flags(int fd)
wait.bo_handle = handle;
wait.timeout_ns = 1;
- wait.flags = 0x;
+ /* NOTE: This test intentionally tests for just the next available flag.
+* Don't "fix" this testcase without the ABI testcases for new flags
+* first. */
+ wait.flags = 1;
ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
igt_assert(ret != 0 && errno == EINVAL);
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Always pass dev pointer in pdp_init
And fix 0-DAY kernel test infrastructure warning.
Reported-by: Fengguang Wu
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 451913a..d2910a8 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1490,7 +1490,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
ppgtt->base.total = 1ULL << 48;
ppgtt->switch_mm = gen8_48b_mm_switch;
} else {
- ret = __pdp_init(false, &ppgtt->pdp);
+ ret = __pdp_init(ppgtt->base.dev, &ppgtt->pdp);
if (ret)
goto free_scratch;
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915/ppgtt: Abstract 4lvl and legacy functions
Cleanup, allocate, insert, clear and dump functions were already vfuncs,
this patch now point them to the 4-level (48-bit) or legacy (32-bit)
versions.
This removes unnecessary checks of which ppgtt is in use, as it now only
happens once, in ppgtt_init.
Suggested-by: Daniel Vetter
Cc: Daniel Vetter
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 253 ++--
1 file changed, 153 insertions(+), 100 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d2910a8..e0d9ae0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -755,28 +755,37 @@ static void gen8_ppgtt_clear_pte_range(struct
i915_address_space *vm,
}
}
-static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
- uint64_t start,
- uint64_t length,
- bool use_scratch)
+static void gen8_legacy_ppgtt_clear_range(struct i915_address_space *vm,
+ uint64_t start,
+ uint64_t length,
+ bool use_scratch)
{
struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base);
gen8_pte_t scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
I915_CACHE_LLC, use_scratch);
- if (!USES_FULL_48BIT_PPGTT(vm->dev)) {
- gen8_ppgtt_clear_pte_range(vm, &ppgtt->pdp, start, length,
- scratch_pte);
- } else {
- uint64_t templ4, pml4e;
- struct i915_page_directory_pointer *pdp;
+ gen8_ppgtt_clear_pte_range(vm, &ppgtt->pdp, start, length,
+ scratch_pte);
+}
- gen8_for_each_pml4e(pdp, &ppgtt->pml4, start, length, templ4,
pml4e) {
- gen8_ppgtt_clear_pte_range(vm, pdp, start, length,
- scratch_pte);
- }
+static void gen8_4lvl_ppgtt_clear_range(struct i915_address_space *vm,
+ uint64_t start,
+ uint64_t length,
+ bool use_scratch)
+{
+ struct i915_hw_ppgtt *ppgtt =
+ container_of(vm, struct i915_hw_ppgtt, base);
+ struct i915_page_directory_pointer *pdp;
+ uint64_t templ4, pml4e;
+ gen8_pte_t scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
+I915_CACHE_LLC, use_scratch);
+
+ gen8_for_each_pml4e(pdp, &ppgtt->pml4, start, length, templ4, pml4e) {
+ gen8_ppgtt_clear_pte_range(vm, pdp, start, length,
+ scratch_pte);
}
+
}
static void
@@ -821,11 +830,11 @@ gen8_ppgtt_insert_pte_entries(struct i915_address_space
*vm,
kunmap_px(ppgtt, pt_vaddr);
}
-static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
- struct sg_table *pages,
- uint64_t start,
- enum i915_cache_level cache_level,
- u32 unused)
+static void gen8_legacy_ppgtt_insert_entries(struct i915_address_space *vm,
+struct sg_table *pages,
+uint64_t start,
+enum i915_cache_level cache_level,
+u32 unused)
{
struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base);
@@ -833,18 +842,28 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
__sg_page_iter_start(&sg_iter, pages->sgl, sg_nents(pages->sgl), 0);
- if (!USES_FULL_48BIT_PPGTT(vm->dev)) {
- gen8_ppgtt_insert_pte_entries(vm, &ppgtt->pdp, &sg_iter, start,
- cache_level);
- } else {
- struct i915_page_directory_pointer *pdp;
- uint64_t templ4, pml4e;
- uint64_t length = (uint64_t)pages->orig_nents << PAGE_SHIFT;
+ gen8_ppgtt_insert_pte_entries(vm, &ppgtt->pdp, &sg_iter, start,
+ cache_level);
+}
- gen8_for_each_pml4e(pdp, &ppgtt->pml4, start, length, templ4,
pml4e) {
- gen8_ppgtt_insert_pte_entries(vm, pdp, &sg_iter,
- start, cache_level);
- }
+static void gen8_4lvl_ppgtt_insert_entries(struct i915_address_space *vm,
+ struct sg_table *pages,
+
[Intel-gfx] [PATCH 2/4] drm/i915: Use complete virtual address range on 32-bit platforms
With the offset length being taken care of in ("drm/i915/gtt: Allow >=
4GB offsets in X86_32"), the code should be finally safe in 32-bit
kernels.
This reverts commit 501fd70fcaebc911b6b96a7b331e6960e5af67e7
Author: Michel Thierry
Date: Fri May 29 14:15:05 2015 +0100
drm/i915: limit PPGTT size to 2GB in 32-bit platforms
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index dee3b39..451913a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1495,14 +1495,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
goto free_scratch;
ppgtt->base.total = 1ULL << 32;
- if (IS_ENABLED(CONFIG_X86_32))
- /* While we have a proliferation of size_t variables
-* we cannot represent the full ppgtt size on 32bit,
-* so limit it to the same size as the GGTT (currently
-* 2GiB).
-*/
- ppgtt->base.total =
to_i915(ppgtt->base.dev)->gtt.base.total;
-
ppgtt->switch_mm = gen8_legacy_mm_switch;
trace_i915_page_directory_pointer_entry_alloc(&ppgtt->base,
0, 0,
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915/gtt: Allow >= 4GB offsets in X86_32
Similar to commit c44ef60e437019b8ca1dab8b4d2e8761fd4ce1e9 ("drm/i915/gtt:
Allow >= 4GB sizes for vm"), i915_gem_obj_offset and i915_gem_obj_ggtt_offset
return an unsigned long, which in only 4-bytes long in 32-bit kernels.
Change return type (and other related offset variables) to u64.
Since Global GTT is always limited to 4GB, this change would not be required
in i915_gem_obj_ggtt_offset, but this is done for consistency.
v2: Remove unnecessary offset variable in do_pin, as we already have
vma->node.start (Chris).
Update GGTT offset too (Tvrtko).
Cc: Chris Wilson
Cc: Daniel Vetter
Cc: Tvrtko Ursulin
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_drv.h | 12 +---
drivers/gpu/drm/i915/i915_gem.c | 18 +++---
drivers/gpu/drm/i915/i915_gem_fence.c | 4 ++--
drivers/gpu/drm/i915/i915_gem_gtt.c | 8
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +-
drivers/gpu/drm/i915/intel_fbdev.c| 2 +-
6 files changed, 20 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bfd6291..4768a93 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2968,13 +2968,11 @@ struct drm_gem_object *i915_gem_prime_import(struct
drm_device *dev,
struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
struct drm_gem_object *gem_obj, int flags);
-unsigned long
-i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
- const struct i915_ggtt_view *view);
-unsigned long
-i915_gem_obj_offset(struct drm_i915_gem_object *o,
- struct i915_address_space *vm);
-static inline unsigned long
+u64 i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
+ const struct i915_ggtt_view *view);
+u64 i915_gem_obj_offset(struct drm_i915_gem_object *o,
+ struct i915_address_space *vm);
+static inline u64
i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
{
return i915_gem_obj_ggtt_offset_view(o, &i915_ggtt_view_normal);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a86227e..b9dde7a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4225,15 +4225,13 @@ i915_gem_object_do_pin(struct drm_i915_gem_object *obj,
return -EBUSY;
if (i915_vma_misplaced(vma, alignment, flags)) {
- unsigned long offset;
- offset = ggtt_view ? i915_gem_obj_ggtt_offset_view(obj,
ggtt_view) :
-i915_gem_obj_offset(obj, vm);
WARN(vma->pin_count,
"bo is already pinned in %s with incorrect
alignment:"
-" offset=%lx, req.alignment=%x,
req.map_and_fenceable=%d,"
+" offset=%08x %08x, req.alignment=%x,
req.map_and_fenceable=%d,"
" obj->map_and_fenceable=%d\n",
ggtt_view ? "ggtt" : "ppgtt",
-offset,
+upper_32_bits(vma->node.start),
+lower_32_bits(vma->node.start),
alignment,
!!(flags & PIN_MAPPABLE),
obj->map_and_fenceable);
@@ -5188,9 +5186,8 @@ void i915_gem_track_fb(struct drm_i915_gem_object *old,
}
/* All the new VM stuff */
-unsigned long
-i915_gem_obj_offset(struct drm_i915_gem_object *o,
- struct i915_address_space *vm)
+u64 i915_gem_obj_offset(struct drm_i915_gem_object *o,
+ struct i915_address_space *vm)
{
struct drm_i915_private *dev_priv = o->base.dev->dev_private;
struct i915_vma *vma;
@@ -5210,9 +5207,8 @@ i915_gem_obj_offset(struct drm_i915_gem_object *o,
return -1;
}
-unsigned long
-i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
- const struct i915_ggtt_view *view)
+u64 i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
+ const struct i915_ggtt_view *view)
{
struct i915_address_space *ggtt = i915_obj_to_ggtt(o);
struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c
b/drivers/gpu/drm/i915/i915_gem_fence.c
index af1f8c4..6077dff 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -128,7 +128,7 @@ static void i915_write_fence_reg(struct drm_device *dev,
int reg,
WARN((i915_gem_obj_ggtt_offset(obj) & ~I915_FENCE_START_MASK) ||
(size & -size) != size ||
(i915_gem_obj_ggtt_offset(obj) & (size - 1)),
-"object 0x%08lx [fenceable? %d] not 1M or pot-size
(0x%08x) aligned\n",
+"object 0x%0
Re: [Intel-gfx] [PATCH] drm/i915: Check idle to active before processing CSQ
On 07/08/2015 12:52, Daniel Vetter wrote: On Fri, Aug 07, 2015 at 11:15:56AM +0300, Mika Kuoppala wrote: Daniel Vetter writes: On Thu, Aug 06, 2015 at 05:09:17PM +0300, Mika Kuoppala wrote: If idle to active bit is set, the rest of the fields in CSQ are not valid. Bail out early if this is the case in order to prevent rest of the loop inspecting stale values. Signed-off-by: Mika Kuoppala looks good to me, didn't observe any impact with this patch. Reviewed-by: Arun Siluvery regards Arun Same questions here too, what's the impact. E.g. if you only found this by bspec/code inspection then it's for -next, but if it's to fix some known breakage then it's for -fixes + cc: stable. To this and the masked write one: Both of these were found when I was trying to find out root cause for skl hangs. They are both for -next. Both are in the correctness department vrt bspec and I haven't observed any other impact. Point taken on being more verbose. Thanks I added a note about this to the first patch and merged it. This one here still seems to miss an r-b. -Daniel ___ 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 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
Re: [Intel-gfx] [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
On Fri, Aug 07, 2015 at 03:53:57PM +0300, David Weinehall wrote:
> On Thu, Aug 06, 2015 at 11:33:00PM +0200, Daniel Vetter wrote:
> > 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.
>
> The reason I never submitted it was probably because of Chris's strong
> opposition to the feature in the first place; I've had the testcase
> laying around on my computer for quite a while.
>
> Anyhow, here's a slightly modified version of that test -- hopefully
> not breaking anything.
>
>
> Signed-off-by: David Weinehall
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90808
btw this is first priority for bug scrub, finding bugs. Also this is a
regression, which actually makes it even more important than just basic
bug scrubbing. Note that bug fixing itself is only about 3rd tier
priority, i.e. something you can do when you have free time by accident.
Oh and the bug is a regression, but not correctly marked as such. That
means QA fail or bug scrub fail, either way we need to figure out what
went wrong here.
Please discuss this with Christophe Prigent as our permanent bug scrub
leader, figure out what needs to be fixed in bkms and present the solution
in next week's bug scrub coordination meeting on Thu.
Thanks, Daniel
>
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index bc5d4bd827cf..f4deca6bd79e 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -102,6 +102,7 @@ struct local_i915_gem_context_param {
> uint32_t size;
> uint64_t param;
> #define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1
> +#define LOCAL_CONTEXT_PARAM_NO_ZEROMAP 0x2
> uint64_t value;
> };
> void gem_context_require_ban_period(int fd);
> diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> index b44b37cf0538..1e7e8ff40703 100644
> --- a/tests/gem_ctx_param_basic.c
> +++ b/tests/gem_ctx_param_basic.c
> @@ -57,7 +57,7 @@ igt_main
> ctx = gem_context_create(fd);
> }
>
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
>
> igt_subtest("basic") {
> ctx_param.context = ctx;
> @@ -98,21 +98,31 @@ igt_main
> ctx_param.size = 0;
> }
>
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
>
> - igt_subtest("invalid-param-get") {
> - ctx_param.context = ctx;
> - TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, EINVAL);
> + igt_subtest("non-root-set") {
> + igt_fork(child, 1) {
> + igt_drop_root();
> +
> + ctx_param.context = ctx;
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> + ctx_param.value--;
> + TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
> + }
> +
> + igt_waitchildren();
> }
>
> - igt_subtest("invalid-param-set") {
> + igt_subtest("root-set") {
> ctx_param.context = ctx;
> - TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> + ctx_param.value--;
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> }
>
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
>
> - igt_subtest("non-root-set") {
> + igt_subtest("non-root-set-no-zeromap") {
> igt_fork(child, 1) {
> igt_drop_root();
>
> @@ -125,13 +135,32 @@ igt_main
> igt_waitchildren();
> }
>
> - igt_subtest("root-set") {
> + igt_subtest("root-set-no-zeromap-enabled") {
> ctx_param.context = ctx;
> TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> - ctx_param.value--;
> + ctx_param.value = 1;
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> + }
> +
> + igt_subtest("root-set-no-zeromap-disabled") {
> + ctx_param.context = ctx;
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> + ctx_param.value = 0;
> TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> }
>
> + ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP + 1;
> +
> + igt_subtest("invalid-param-get") {
> + ctx_param.context = ctx;
> + TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, EINVAL);
> +
Re: [Intel-gfx] [PATCH] drm/i915: Only move to the CPU write domain if keeping the GTT pages
On Fri, Aug 07, 2015 at 01:55:01PM +0200, Daniel Vetter wrote: > On Fri, Aug 07, 2015 at 11:10:58AM +0100, Chris Wilson wrote: > > On Fri, Aug 07, 2015 at 10:07:28AM +0200, Daniel Vetter wrote: > > > On Thu, Aug 06, 2015 at 05:43:39PM +0100, Chris Wilson wrote: > > > But it's still salvageable I think since we only care about coherency for > > > the gpu (where data might be stuck in cpu caches). From the cpu's pov (and > > > hence the entire system except the gpu) we should never see inconsistency > > > really - as soon as the gpu does a write to a cacheline it'll win, and > > > before that nothing in the system can assume anything about the contents > > > of these pages. > > > > But the GPU doesn't write to cachelines (except in LLC/snooped+flush). > > The issue is what happens when the user lies about writing to the object > > through a WB cpu mapping (dirtying a cacheline) and the GPU also does. > > Who wins then? > > > > We have postulated that it could be entirely possible for the CPU to > > trust it cache and return local contents and for those to be also > > considered not dirty and so not flushed to memory. Later, we then read > > what the gpu wrote and choas ensues. > > This was just with an eye towards purged memory where we don't care about > correct data anyway. The only thing we care about is that when it's all > overwritten again by someone, that someone should win. And since GEM > assumes new pages are in the cpu domain and clflushes them first that > should hold even for GEM. But the tricky part is that I think we can pull > this off only if the backing storage is purged already. But what's the difference between: lock put_pages purge and lock purge put_pages if you are dismissing the user dirtying CPU cachelines vs already dirty GPU data as being a source of worry? -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 i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
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.
>
> igt_subtest("invalid-param-get") {
> ctx_param.context = ctx;
> --
> 2.5.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
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 Thu, Aug 06, 2015 at 11:33:00PM +0200, Daniel Vetter wrote:
> 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.
The reason I never submitted it was probably because of Chris's strong
opposition to the feature in the first place; I've had the testcase
laying around on my computer for quite a while.
Anyhow, here's a slightly modified version of that test -- hopefully
not breaking anything.
Signed-off-by: David Weinehall
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index bc5d4bd827cf..f4deca6bd79e 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -102,6 +102,7 @@ struct local_i915_gem_context_param {
uint32_t size;
uint64_t param;
#define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1
+#define LOCAL_CONTEXT_PARAM_NO_ZEROMAP 0x2
uint64_t value;
};
void gem_context_require_ban_period(int fd);
diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
index b44b37cf0538..1e7e8ff40703 100644
--- a/tests/gem_ctx_param_basic.c
+++ b/tests/gem_ctx_param_basic.c
@@ -57,7 +57,7 @@ igt_main
ctx = gem_context_create(fd);
}
- ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+ ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
igt_subtest("basic") {
ctx_param.context = ctx;
@@ -98,21 +98,31 @@ igt_main
ctx_param.size = 0;
}
- ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
+ ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
- igt_subtest("invalid-param-get") {
- ctx_param.context = ctx;
- TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, EINVAL);
+ igt_subtest("non-root-set") {
+ igt_fork(child, 1) {
+ igt_drop_root();
+
+ ctx_param.context = ctx;
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
+ ctx_param.value--;
+ TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
+ }
+
+ igt_waitchildren();
}
- igt_subtest("invalid-param-set") {
+ igt_subtest("root-set") {
ctx_param.context = ctx;
- TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
+ ctx_param.value--;
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
}
- ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+ ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
- igt_subtest("non-root-set") {
+ igt_subtest("non-root-set-no-zeromap") {
igt_fork(child, 1) {
igt_drop_root();
@@ -125,13 +135,32 @@ igt_main
igt_waitchildren();
}
- igt_subtest("root-set") {
+ igt_subtest("root-set-no-zeromap-enabled") {
ctx_param.context = ctx;
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
- ctx_param.value--;
+ ctx_param.value = 1;
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
+ }
+
+ igt_subtest("root-set-no-zeromap-disabled") {
+ ctx_param.context = ctx;
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
+ ctx_param.value = 0;
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
}
+ ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP + 1;
+
+ igt_subtest("invalid-param-get") {
+ ctx_param.context = ctx;
+ TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, EINVAL);
+ }
+
+ igt_subtest("invalid-param-set") {
+ ctx_param.context = ctx;
+ TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
+ }
+
igt_fixture
close(fd);
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/vblank: Use u32 consistently for vblank counters
On Fri, Aug 07, 2015 at 12:31:17PM +0200, Daniel Vetter wrote: > In > > commit 99264a61dfcda41d86d0960cf2d4c0fc2758a773 > Author: Daniel Vetter > Date: Wed Apr 15 19:34:43 2015 +0200 > > drm/vblank: Fixup and document timestamp update/read barriers > > I've switched vblank->count from atomic_t to unsigned long and > accidentally created an integer comparison bug in > drm_vblank_count_and_time since vblanke->count might overflow the u32 > local copy and hence the retry loop never succeed. > > Fix this by consistently using u32. > > Cc: Michel Dänzer > Reported-by: Michel Dänzer > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_irq.c | 2 +- > include/drm/drmP.h| 2 +- > 2 files changed, 2 insertions(+), 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] drm/i915: Only move to the CPU write domain if keeping the GTT pages
On Fri, Aug 07, 2015 at 11:10:58AM +0100, Chris Wilson wrote: > On Fri, Aug 07, 2015 at 10:07:28AM +0200, Daniel Vetter wrote: > > On Thu, Aug 06, 2015 at 05:43:39PM +0100, Chris Wilson wrote: > > But it's still salvageable I think since we only care about coherency for > > the gpu (where data might be stuck in cpu caches). From the cpu's pov (and > > hence the entire system except the gpu) we should never see inconsistency > > really - as soon as the gpu does a write to a cacheline it'll win, and > > before that nothing in the system can assume anything about the contents > > of these pages. > > But the GPU doesn't write to cachelines (except in LLC/snooped+flush). > The issue is what happens when the user lies about writing to the object > through a WB cpu mapping (dirtying a cacheline) and the GPU also does. > Who wins then? > > We have postulated that it could be entirely possible for the CPU to > trust it cache and return local contents and for those to be also > considered not dirty and so not flushed to memory. Later, we then read > what the gpu wrote and choas ensues. This was just with an eye towards purged memory where we don't care about correct data anyway. The only thing we care about is that when it's all overwritten again by someone, that someone should win. And since GEM assumes new pages are in the cpu domain and clflushes them first that should hold even for GEM. But the tricky part is that I think we can pull this off only if the backing storage is purged already. -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/i915: Check idle to active before processing CSQ
On Fri, Aug 07, 2015 at 11:15:56AM +0300, Mika Kuoppala wrote: > Daniel Vetter writes: > > > On Thu, Aug 06, 2015 at 05:09:17PM +0300, Mika Kuoppala wrote: > >> If idle to active bit is set, the rest of the fields > >> in CSQ are not valid. > >> > >> Bail out early if this is the case in order to prevent > >> rest of the loop inspecting stale values. > >> > >> Signed-off-by: Mika Kuoppala > > > > Same questions here too, what's the impact. E.g. if you only found this by > > bspec/code inspection then it's for -next, but if it's to fix some known > > breakage then it's for -fixes + cc: stable. > > > > To this and the masked write one: Both of these were found > when I was trying to find out root cause for skl hangs. > > They are both for -next. Both are in the correctness > department vrt bspec and I haven't observed any other > impact. > > Point taken on being more verbose. Thanks I added a note about this to the first patch and merged it. This one here still seems to miss an 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] [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer
On Fri, Aug 7, 2015 at 12:21 PM, Chris Wilson wrote:
> On Fri, Aug 07, 2015 at 10:21:34AM +0100, Michel Thierry wrote:
>> On 8/6/2015 11:00 PM, Daniel Vetter wrote:
>> >On Thu, Aug 6, 2015 at 10:17 PM, kbuild test robot
>> > wrote:
>> >> 1070 if (IS_ENABLED(CONFIG_X86_32))
>> >> 1071 /* While we have a proliferation of size_t
>> >> variables
>> >> 1072 * we cannot represent the full ppgtt size on
>> >> 32bit,
>> >> 1073 * so limit it to the same size as the GGTT
>> >> (currently
>> >> 1074 * 2GiB).
>> >> 1075 */
>> >> 1076 ppgtt->base.total =
>> >> to_i915(ppgtt->base.dev)->gtt.base.total;
>> >> 1077 ppgtt->base.cleanup = gen8_ppgtt_cleanup;
>> >> 1078 ppgtt->base.allocate_va_range = gen8_alloc_va_range;
>> >> 1079 ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
>> >> 1080 ppgtt->base.clear_range = gen8_ppgtt_clear_range;
>> >> 1081 ppgtt->base.unbind_vma = ppgtt_unbind_vma;
>> >> 1082 ppgtt->base.bind_vma = ppgtt_bind_vma;
>> >> 1083
>> >> 1084 ppgtt->switch_mm = gen8_mm_switch;
>> >> 1085
>> >>>1086 ret = __pdp_init(false, &ppgtt->pdp);
>> >
>> >So the first argument of pdp_init ist struct drm_device *dev and yes
>> >the first thing it does is deref it.
>> >
>>
>> *dev is used only for I915_PDPES_PER_PDP/USES_FULL_48BIT_PPGTT,
>> which in this path is always false. I didn't expect kbuild to
>> complain. I'll change it with the other modifications I'm about to
>> send.
>
> Wrong. dev is never deferenced even though it is passed around.
Yeah I spotted that too right after sending out my mail.
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 41c5e7c9c8ab..37283d5a8a4a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2549,12 +2549,12 @@ struct drm_i915_cmd_table {
>
> #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
> #define HAS_LOGICAL_RING_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 8)
> -#define USES_PPGTT(dev)(i915.enable_ppgtt)
> -#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt >= 2)
> +#define USES_PPGTT() (i915.enable_ppgtt)
> +#define USES_FULL_PPGTT() (i915.enable_ppgtt >= 2)
> #ifdef CONFIG_X86_64
> -# define USES_FULL_48BIT_PPGTT(dev)(i915.enable_ppgtt == 3)
> +# define USES_FULL_48BIT_PPGTT() (i915.enable_ppgtt == 3)
> #else
> -# define USES_FULL_48BIT_PPGTT(dev)false
> +# define USES_FULL_48BIT_PPGTT() false
> #endif
I'd vote to abolish all these macros and just add an enum for ppgtt modes:
enum i915_ppgtt_mode {
PPGTT_DISABLED = 0,
ALIASING_PPGTT = 1,
FULL_PPGTT = 2,
FULL_48B_PPGTT = 3 };
Then just open-code the checks. Of course separate patch from anything
else really.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH libdrm v3 1/2] intel: Add EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag
On 8/7/2015 11:56 AM, Michał Winiarski wrote:
On Fri, Aug 07, 2015 at 10:45:21AM +0100, 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 (GSH) or Instruction State Heap (ISH) must be in a
32-bit range, because the General State Offset and Instruction State Offset
are limited to 32-bits.
The i915 driver has been modified to provide a flag to set when the 4GB
limit is not necessary in a given bo (EXEC_OBJECT_SUPPORTS_48B_ADDRESS).
48-bit range will only be used when explicitly requested.
Calls to the new drm_intel_bo_emit_reloc_48bit function will have this flag
set automatically, while calls to drm_intel_bo_emit_reloc will clear it.
v2: Make set/clear functions nops on pre-gen8 platforms, and use them
internally in emit_reloc functions (Ben)
s/48BADDRESS/48B_ADDRESS/ (Dave)
v3: Keep set/clear functions internal, no-one needs to use them directly.
References:
http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html
Cc: Ben Widawsky
Cc: Dave Gordon
Signed-off-by: Michel Thierry
---
include/drm/i915_drm.h| 3 ++-
intel/intel_bufmgr.c | 16 ++
intel/intel_bufmgr.h | 6 +-
intel/intel_bufmgr_gem.c | 54 +++
intel/intel_bufmgr_priv.h | 11 ++
5 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index ded43b1..426b25c 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -680,7 +680,8 @@ struct drm_i915_gem_exec_object2 {
#define EXEC_OBJECT_NEEDS_FENCE (1<<0)
#define EXEC_OBJECT_NEEDS_GTT (1<<1)
#define EXEC_OBJECT_WRITE (1<<2)
-#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_WRITE<<1)
+#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
+#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_SUPPORTS_48B_ADDRESS<<1)
__u64 flags;
__u64 rsvd1;
diff --git a/intel/intel_bufmgr.c b/intel/intel_bufmgr.c
index 14ea9f9..0bd5191 100644
--- a/intel/intel_bufmgr.c
+++ b/intel/intel_bufmgr.c
@@ -202,6 +202,22 @@ drm_intel_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset,
drm_intel_bo *target_bo, uint32_t target_offset,
uint32_t read_domains, uint32_t write_domain)
{
+ if (bo->bufmgr->bo_clear_supports_48b_address)
+ bo->bufmgr->bo_clear_supports_48b_address(target_bo);
This is always true - you assign func to this func pointer unconditionally.
Also - why not return some meaningful value if the user does not have
enable_ppgtt=3 set? You can get that from I915_PARAM_HAS_ALIASING_PPGTT right
now. Check for gen >= 8 seems rather inadequate, and even then you're not
returning anything useful to the caller.
+
+ return bo->bufmgr->bo_emit_reloc(bo, offset,
+target_bo, target_offset,
+read_domains, write_domain);
+}
+
Using emit_reloc to set a BO flag seems confusing and can be error prone,
emit_reloc can be called many times and caller needs to be careful and call the
right function for each reloc.
+int
+drm_intel_bo_emit_reloc_48bit(drm_intel_bo *bo, uint32_t offset,
+ drm_intel_bo *target_bo, uint32_t target_offset,
+ uint32_t read_domains, uint32_t write_domain)
+{
+ if (bo->bufmgr->bo_set_supports_48b_address)
+ bo->bufmgr->bo_set_supports_48b_address(target_bo);
Same situation as with clear_supports_48b_address.
+
return bo->bufmgr->bo_emit_reloc(bo, offset,
target_bo, target_offset,
read_domains, write_domain);
diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
index 285919e..8f91ffe 100644
--- a/intel/intel_bufmgr.h
+++ b/intel/intel_bufmgr.h
@@ -87,7 +87,8 @@ struct _drm_intel_bo {
/**
* Last seen card virtual address (offset from the beginning of the
* aperture) for the object. This should be used to fill relocation
-* entries when calling drm_intel_bo_emit_reloc()
+* entries when calling drm_intel_bo_emit_reloc() or
+* drm_intel_bo_emit_reloc_48bit()
*/
uint64_t offset64;
};
@@ -147,6 +148,9 @@ int drm_intel_bufmgr_check_aperture_space(drm_intel_bo **
bo_array, int count);
int drm_intel_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset,
drm_intel_bo *target_bo, uint32_t target_offset,
uint32_t read_domains, uint32_t write_domain);
+int drm_intel_bo_emit_reloc_48bit(drm_intel_bo *bo, uint32_t offset,
+ drm_intel_bo *target_bo, uint32_t
target_offset,
+ uint32_t read_domains, uint32_t write_domain);
int drm_intel_bo_e
[Intel-gfx] [RFC libdrm] intel: 48b ppgtt support
---
include/drm/i915_drm.h| 3 ++-
intel/intel_bufmgr.c | 11 +++
intel/intel_bufmgr.h | 1 +
intel/intel_bufmgr_gem.c | 43 +--
intel/intel_bufmgr_priv.h | 8
5 files changed, 55 insertions(+), 11 deletions(-)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index ded43b1..426b25c 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -680,7 +680,8 @@ struct drm_i915_gem_exec_object2 {
#define EXEC_OBJECT_NEEDS_FENCE (1<<0)
#define EXEC_OBJECT_NEEDS_GTT (1<<1)
#define EXEC_OBJECT_WRITE (1<<2)
-#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_WRITE<<1)
+#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
+#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_SUPPORTS_48B_ADDRESS<<1)
__u64 flags;
__u64 rsvd1;
diff --git a/intel/intel_bufmgr.c b/intel/intel_bufmgr.c
index 14ea9f9..97ea6ec 100644
--- a/intel/intel_bufmgr.c
+++ b/intel/intel_bufmgr.c
@@ -261,6 +261,17 @@ drm_intel_bo_get_tiling(drm_intel_bo *bo, uint32_t *
tiling_mode,
}
int
+drm_intel_bo_use_full_range(drm_intel_bo *bo, uint32_t enable)
+{
+ if (bo->bufmgr->bo_use_full_range) {
+ bo->bufmgr->bo_use_full_range(bo, enable);
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
+int
drm_intel_bo_disable_reuse(drm_intel_bo *bo)
{
if (bo->bufmgr->bo_disable_reuse)
diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
index 285919e..2635fa4 100644
--- a/intel/intel_bufmgr.h
+++ b/intel/intel_bufmgr.h
@@ -160,6 +160,7 @@ int drm_intel_bo_get_tiling(drm_intel_bo *bo, uint32_t *
tiling_mode,
int drm_intel_bo_flink(drm_intel_bo *bo, uint32_t * name);
int drm_intel_bo_busy(drm_intel_bo *bo);
int drm_intel_bo_madvise(drm_intel_bo *bo, int madv);
+int drm_intel_bo_use_full_range(drm_intel_bo *bo, uint32_t enable);
int drm_intel_bo_disable_reuse(drm_intel_bo *bo);
int drm_intel_bo_is_reusable(drm_intel_bo *bo);
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 41de396..ef71686 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -237,6 +237,11 @@ struct _drm_intel_bo_gem {
bool is_userptr;
/**
+* Whether this buffer can be placed in full (2^48) ppgtt range on gen8+
+*/
+ bool uses_full_range;
+
+ /**
* Size in bytes of this buffer and its relocation descendents.
*
* Used to avoid costly tree walking in
@@ -395,8 +400,8 @@ drm_intel_gem_dump_validation_list(drm_intel_bufmgr_gem
*bufmgr_gem)
drm_intel_bo_gem *target_gem =
(drm_intel_bo_gem *) target_bo;
- DBG("%2d: %d (%s)@0x%08llx -> "
- "%d (%s)@0x%08lx + 0x%08x\n",
+ DBG("%2d: %d (%s)@0x%016llx -> "
+ "%d (%s)@0x%016lx + 0x%08x\n",
i,
bo_gem->gem_handle, bo_gem->name,
(unsigned long long)bo_gem->relocs[j].offset,
@@ -468,11 +473,15 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int
need_fence)
drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo->bufmgr;
drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *)bo;
int index;
+ int flags = 0;
+
+ if (need_fence)
+ flags |= EXEC_OBJECT_NEEDS_FENCE;
+ if (bo_gem->uses_full_range)
+ flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
if (bo_gem->validate_index != -1) {
- if (need_fence)
- bufmgr_gem->exec2_objects[bo_gem->validate_index].flags
|=
- EXEC_OBJECT_NEEDS_FENCE;
+ bufmgr_gem->exec2_objects[bo_gem->validate_index].flags |=
flags;
return;
}
@@ -501,13 +510,9 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int
need_fence)
bufmgr_gem->exec2_objects[index].alignment = bo->align;
bufmgr_gem->exec2_objects[index].offset = 0;
bufmgr_gem->exec_bos[index] = bo;
- bufmgr_gem->exec2_objects[index].flags = 0;
+ bufmgr_gem->exec2_objects[index].flags = flags;
bufmgr_gem->exec2_objects[index].rsvd1 = 0;
bufmgr_gem->exec2_objects[index].rsvd2 = 0;
- if (need_fence) {
- bufmgr_gem->exec2_objects[index].flags |=
- EXEC_OBJECT_NEEDS_FENCE;
- }
bufmgr_gem->exec_count++;
}
@@ -780,6 +785,7 @@ retry:
bo_gem->used_as_reloc_target = false;
bo_gem->has_error = false;
bo_gem->reusable = true;
+ bo_gem->uses_full_range = false;
drm_intel_bo_gem_set_in_aperture_size(bufmgr_gem, bo_gem, alignment);
@@ -926,6 +932,7 @@ drm_intel_gem_bo_alloc_userptr(drm_intel_bufmgr *bufmgr,
bo_gem->used_as_reloc_target = false;
bo_gem->has_error = false;
bo_gem->reusable = false;
+ bo_gem->uses_full_range = fa
Re: [Intel-gfx] [PATCH libdrm v3 1/2] intel: Add EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag
On Fri, Aug 07, 2015 at 10:45:21AM +0100, 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 (GSH) or Instruction State Heap (ISH) must be in a
> 32-bit range, because the General State Offset and Instruction State Offset
> are limited to 32-bits.
>
> The i915 driver has been modified to provide a flag to set when the 4GB
> limit is not necessary in a given bo (EXEC_OBJECT_SUPPORTS_48B_ADDRESS).
> 48-bit range will only be used when explicitly requested.
>
> Calls to the new drm_intel_bo_emit_reloc_48bit function will have this flag
> set automatically, while calls to drm_intel_bo_emit_reloc will clear it.
>
> v2: Make set/clear functions nops on pre-gen8 platforms, and use them
> internally in emit_reloc functions (Ben)
> s/48BADDRESS/48B_ADDRESS/ (Dave)
> v3: Keep set/clear functions internal, no-one needs to use them directly.
>
> References:
> http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html
> Cc: Ben Widawsky
> Cc: Dave Gordon
> Signed-off-by: Michel Thierry
> ---
> include/drm/i915_drm.h| 3 ++-
> intel/intel_bufmgr.c | 16 ++
> intel/intel_bufmgr.h | 6 +-
> intel/intel_bufmgr_gem.c | 54
> +++
> intel/intel_bufmgr_priv.h | 11 ++
> 5 files changed, 84 insertions(+), 6 deletions(-)
>
> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index ded43b1..426b25c 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -680,7 +680,8 @@ struct drm_i915_gem_exec_object2 {
> #define EXEC_OBJECT_NEEDS_FENCE (1<<0)
> #define EXEC_OBJECT_NEEDS_GTT(1<<1)
> #define EXEC_OBJECT_WRITE(1<<2)
> -#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_WRITE<<1)
> +#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
> +#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_SUPPORTS_48B_ADDRESS<<1)
> __u64 flags;
>
> __u64 rsvd1;
> diff --git a/intel/intel_bufmgr.c b/intel/intel_bufmgr.c
> index 14ea9f9..0bd5191 100644
> --- a/intel/intel_bufmgr.c
> +++ b/intel/intel_bufmgr.c
> @@ -202,6 +202,22 @@ drm_intel_bo_emit_reloc(drm_intel_bo *bo, uint32_t
> offset,
> drm_intel_bo *target_bo, uint32_t target_offset,
> uint32_t read_domains, uint32_t write_domain)
> {
> + if (bo->bufmgr->bo_clear_supports_48b_address)
> + bo->bufmgr->bo_clear_supports_48b_address(target_bo);
This is always true - you assign func to this func pointer unconditionally.
Also - why not return some meaningful value if the user does not have
enable_ppgtt=3 set? You can get that from I915_PARAM_HAS_ALIASING_PPGTT right
now. Check for gen >= 8 seems rather inadequate, and even then you're not
returning anything useful to the caller.
> +
> + return bo->bufmgr->bo_emit_reloc(bo, offset,
> + target_bo, target_offset,
> + read_domains, write_domain);
> +}
> +
Using emit_reloc to set a BO flag seems confusing and can be error prone,
emit_reloc can be called many times and caller needs to be careful and call the
right function for each reloc.
> +int
> +drm_intel_bo_emit_reloc_48bit(drm_intel_bo *bo, uint32_t offset,
> + drm_intel_bo *target_bo, uint32_t target_offset,
> + uint32_t read_domains, uint32_t write_domain)
> +{
> + if (bo->bufmgr->bo_set_supports_48b_address)
> + bo->bufmgr->bo_set_supports_48b_address(target_bo);
Same situation as with clear_supports_48b_address.
> +
> return bo->bufmgr->bo_emit_reloc(bo, offset,
>target_bo, target_offset,
>read_domains, write_domain);
> diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
> index 285919e..8f91ffe 100644
> --- a/intel/intel_bufmgr.h
> +++ b/intel/intel_bufmgr.h
> @@ -87,7 +87,8 @@ struct _drm_intel_bo {
> /**
>* Last seen card virtual address (offset from the beginning of the
>* aperture) for the object. This should be used to fill relocation
> - * entries when calling drm_intel_bo_emit_reloc()
> + * entries when calling drm_intel_bo_emit_reloc() or
> + * drm_intel_bo_emit_reloc_48bit()
>*/
> uint64_t offset64;
> };
> @@ -147,6 +148,9 @@ int drm_intel_bufmgr_check_aperture_space(drm_intel_bo **
> bo_array, int count);
> int drm_intel_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset,
> drm_intel_bo *target_bo, uint32_t target_offset,
> uint32_t read_domains, uint32_t write_domain);
> +int drm_intel_bo_emit_reloc_48bit(drm_intel_bo *bo, uint32_t offset,
> + drm_intel_bo *target_bo, uint32_t
> target_offset,
> +
[Intel-gfx] [PATCH] drm/vblank: Use u32 consistently for vblank counters
In
commit 99264a61dfcda41d86d0960cf2d4c0fc2758a773
Author: Daniel Vetter
Date: Wed Apr 15 19:34:43 2015 +0200
drm/vblank: Fixup and document timestamp update/read barriers
I've switched vblank->count from atomic_t to unsigned long and
accidentally created an integer comparison bug in
drm_vblank_count_and_time since vblanke->count might overflow the u32
local copy and hence the retry loop never succeed.
Fix this by consistently using u32.
Cc: Michel Dänzer
Reported-by: Michel Dänzer
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c | 2 +-
include/drm/drmP.h| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index a7311c6e6f30..a2cfd2cc8c39 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -75,7 +75,7 @@ module_param_named(timestamp_precision_usec,
drm_timestamp_precision, int, 0600)
module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600);
static void store_vblank(struct drm_device *dev, int crtc,
-unsigned vblank_count_inc,
+u32 vblank_count_inc,
struct timeval *t_vblank)
{
struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fdf8a501cb44..38e0e77d7277 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -691,7 +691,7 @@ struct drm_vblank_crtc {
struct timer_list disable_timer;/* delayed disable
timer */
/* vblank counter, protected by dev->vblank_time_lock for writes */
- unsigned long count;
+ u32 count;
/* vblank timestamps, protected by dev->vblank_time_lock for writes */
struct timeval time[DRM_VBLANKTIME_RBSIZE];
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer
On Fri, Aug 07, 2015 at 10:21:34AM +0100, Michel Thierry wrote:
> On 8/6/2015 11:00 PM, Daniel Vetter wrote:
> >On Thu, Aug 6, 2015 at 10:17 PM, kbuild test robot
> > wrote:
> >> 1070 if (IS_ENABLED(CONFIG_X86_32))
> >> 1071 /* While we have a proliferation of size_t
> >> variables
> >> 1072 * we cannot represent the full ppgtt size on
> >> 32bit,
> >> 1073 * so limit it to the same size as the GGTT
> >> (currently
> >> 1074 * 2GiB).
> >> 1075 */
> >> 1076 ppgtt->base.total =
> >> to_i915(ppgtt->base.dev)->gtt.base.total;
> >> 1077 ppgtt->base.cleanup = gen8_ppgtt_cleanup;
> >> 1078 ppgtt->base.allocate_va_range = gen8_alloc_va_range;
> >> 1079 ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
> >> 1080 ppgtt->base.clear_range = gen8_ppgtt_clear_range;
> >> 1081 ppgtt->base.unbind_vma = ppgtt_unbind_vma;
> >> 1082 ppgtt->base.bind_vma = ppgtt_bind_vma;
> >> 1083
> >> 1084 ppgtt->switch_mm = gen8_mm_switch;
> >> 1085
> >>>1086 ret = __pdp_init(false, &ppgtt->pdp);
> >
> >So the first argument of pdp_init ist struct drm_device *dev and yes
> >the first thing it does is deref it.
> >
>
> *dev is used only for I915_PDPES_PER_PDP/USES_FULL_48BIT_PPGTT,
> which in this path is always false. I didn't expect kbuild to
> complain. I'll change it with the other modifications I'm about to
> send.
Wrong. dev is never deferenced even though it is passed around.
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 41c5e7c9c8ab..37283d5a8a4a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2549,12 +2549,12 @@ struct drm_i915_cmd_table {
#define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
#define HAS_LOGICAL_RING_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 8)
-#define USES_PPGTT(dev)(i915.enable_ppgtt)
-#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt >= 2)
+#define USES_PPGTT() (i915.enable_ppgtt)
+#define USES_FULL_PPGTT() (i915.enable_ppgtt >= 2)
#ifdef CONFIG_X86_64
-# define USES_FULL_48BIT_PPGTT(dev)(i915.enable_ppgtt == 3)
+# define USES_FULL_48BIT_PPGTT() (i915.enable_ppgtt == 3)
#else
-# define USES_FULL_48BIT_PPGTT(dev)false
+# define USES_FULL_48BIT_PPGTT() false
#endif
-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] drm/i915: Clean up lrc context init
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"?
-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] drm/i915: Only move to the CPU write domain if keeping the GTT pages
On Fri, Aug 07, 2015 at 10:07:28AM +0200, Daniel Vetter wrote: > On Thu, Aug 06, 2015 at 05:43:39PM +0100, Chris Wilson wrote: > But it's still salvageable I think since we only care about coherency for > the gpu (where data might be stuck in cpu caches). From the cpu's pov (and > hence the entire system except the gpu) we should never see inconsistency > really - as soon as the gpu does a write to a cacheline it'll win, and > before that nothing in the system can assume anything about the contents > of these pages. But the GPU doesn't write to cachelines (except in LLC/snooped+flush). The issue is what happens when the user lies about writing to the object through a WB cpu mapping (dirtying a cacheline) and the GPU also does. Who wins then? We have postulated that it could be entirely possible for the CPU to trust it cache and return local contents and for those to be also considered not dirty and so not flushed to memory. Later, we then read what the gpu wrote and choas ensues. -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
[Intel-gfx] [PATCH] drm/i915: Clean up lrc context init
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.
Issue: VIZ-4798
Signed-off-by: Nick Hoath
---
drivers/gpu/drm/i915/i915_drv.h| 1 -
drivers/gpu/drm/i915/i915_gem.c| 23 ++---
drivers/gpu/drm/i915/i915_gem_context.c| 21 -
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 36 +++-
drivers/gpu/drm/i915/intel_lrc.c | 133 +++--
drivers/gpu/drm/i915/intel_lrc.h | 4 +-
drivers/gpu/drm/i915/intel_ringbuffer.c| 2 +
drivers/gpu/drm/i915/intel_ringbuffer.h| 1 +
8 files changed, 105 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 92ea9ad..aa6c6dc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3065,7 +3065,6 @@ int __must_check i915_gem_context_init(struct drm_device
*dev);
void i915_gem_context_fini(struct drm_device *dev);
void i915_gem_context_reset(struct drm_device *dev);
int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
-int i915_gem_context_enable(struct drm_i915_gem_request *req);
void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
int i915_switch_context(struct drm_i915_gem_request *req);
struct intel_context *
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d9f2701..7ebc6e3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4996,14 +4996,8 @@ int i915_gem_init_rings(struct drm_device *dev)
goto cleanup_vebox_ring;
}
- ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
- if (ret)
- goto cleanup_bsd2_ring;
-
return 0;
-cleanup_bsd2_ring:
- intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
cleanup_vebox_ring:
intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
cleanup_blt_ring:
@@ -5022,6 +5016,7 @@ i915_gem_init_hw(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring;
int ret, i, j;
+ struct drm_i915_gem_request *req;
if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
return -EIO;
@@ -5073,9 +5068,12 @@ i915_gem_init_hw(struct drm_device *dev)
goto out;
}
+ ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
+ if (ret)
+ goto out;
+
/* Now it is safe to go back round and do everything else: */
for_each_ring(ring, dev_priv, i) {
- struct drm_i915_gem_request *req;
WARN_ON(!ring->default_context);
@@ -5098,9 +5096,14 @@ i915_gem_init_hw(struct drm_device *dev)
goto out;
}
- ret = i915_gem_context_enable(req);
- if (ret && ret != -EIO) {
- DRM_ERROR("Context enable ring #%d failed %d\n", i,
ret);
+ ret = 0;
+ if (ring->switch_context != NULL) {
+ ret = ring->switch_context(req);
+ } else if (ring->init_context != NULL) {
+ ret = ring->init_context(req);
+ }
+ if (ret) {
+ DRM_ERROR("ring init context: %d\n", ret);
i915_gem_request_cancel(req);
i915_gem_cleanup_ringbuffer(dev);
goto out;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index b77a8f7..18bd5f5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -407,27 +407,6 @@ void i915_gem_context_fini(struct drm_device *dev)
i915_gem_context_unreference(dctx);
}
-int i915_gem_context_enable(struct drm_i915_gem_request *req)
-{
- struct intel_engine_cs *ring = req->ring;
- int ret;
-
- if (i915.enable_execlists) {
- if (ring->init_context == NULL)
- return 0;
-
- ret = ring->init_context(req);
- } else
- ret = i915_switch_context(req);
-
- if (ret) {
- DRM_ERROR("ring init context: %d\n", ret);
- return ret;
- }
-
- return 0;
-}
-
static int context_idr_cleanup(int id, void *p, void *data)
{
struct intel_context *ctx = p;
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(struc
[Intel-gfx] [PATCH libdrm v3 1/2] intel: Add EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag
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 (GSH) or Instruction State Heap (ISH) must be in a
32-bit range, because the General State Offset and Instruction State Offset
are limited to 32-bits.
The i915 driver has been modified to provide a flag to set when the 4GB
limit is not necessary in a given bo (EXEC_OBJECT_SUPPORTS_48B_ADDRESS).
48-bit range will only be used when explicitly requested.
Calls to the new drm_intel_bo_emit_reloc_48bit function will have this flag
set automatically, while calls to drm_intel_bo_emit_reloc will clear it.
v2: Make set/clear functions nops on pre-gen8 platforms, and use them
internally in emit_reloc functions (Ben)
s/48BADDRESS/48B_ADDRESS/ (Dave)
v3: Keep set/clear functions internal, no-one needs to use them directly.
References:
http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html
Cc: Ben Widawsky
Cc: Dave Gordon
Signed-off-by: Michel Thierry
---
include/drm/i915_drm.h| 3 ++-
intel/intel_bufmgr.c | 16 ++
intel/intel_bufmgr.h | 6 +-
intel/intel_bufmgr_gem.c | 54 +++
intel/intel_bufmgr_priv.h | 11 ++
5 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index ded43b1..426b25c 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -680,7 +680,8 @@ struct drm_i915_gem_exec_object2 {
#define EXEC_OBJECT_NEEDS_FENCE (1<<0)
#define EXEC_OBJECT_NEEDS_GTT (1<<1)
#define EXEC_OBJECT_WRITE (1<<2)
-#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_WRITE<<1)
+#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
+#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_SUPPORTS_48B_ADDRESS<<1)
__u64 flags;
__u64 rsvd1;
diff --git a/intel/intel_bufmgr.c b/intel/intel_bufmgr.c
index 14ea9f9..0bd5191 100644
--- a/intel/intel_bufmgr.c
+++ b/intel/intel_bufmgr.c
@@ -202,6 +202,22 @@ drm_intel_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset,
drm_intel_bo *target_bo, uint32_t target_offset,
uint32_t read_domains, uint32_t write_domain)
{
+ if (bo->bufmgr->bo_clear_supports_48b_address)
+ bo->bufmgr->bo_clear_supports_48b_address(target_bo);
+
+ return bo->bufmgr->bo_emit_reloc(bo, offset,
+target_bo, target_offset,
+read_domains, write_domain);
+}
+
+int
+drm_intel_bo_emit_reloc_48bit(drm_intel_bo *bo, uint32_t offset,
+ drm_intel_bo *target_bo, uint32_t target_offset,
+ uint32_t read_domains, uint32_t write_domain)
+{
+ if (bo->bufmgr->bo_set_supports_48b_address)
+ bo->bufmgr->bo_set_supports_48b_address(target_bo);
+
return bo->bufmgr->bo_emit_reloc(bo, offset,
target_bo, target_offset,
read_domains, write_domain);
diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
index 285919e..8f91ffe 100644
--- a/intel/intel_bufmgr.h
+++ b/intel/intel_bufmgr.h
@@ -87,7 +87,8 @@ struct _drm_intel_bo {
/**
* Last seen card virtual address (offset from the beginning of the
* aperture) for the object. This should be used to fill relocation
-* entries when calling drm_intel_bo_emit_reloc()
+* entries when calling drm_intel_bo_emit_reloc() or
+* drm_intel_bo_emit_reloc_48bit()
*/
uint64_t offset64;
};
@@ -147,6 +148,9 @@ int drm_intel_bufmgr_check_aperture_space(drm_intel_bo **
bo_array, int count);
int drm_intel_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset,
drm_intel_bo *target_bo, uint32_t target_offset,
uint32_t read_domains, uint32_t write_domain);
+int drm_intel_bo_emit_reloc_48bit(drm_intel_bo *bo, uint32_t offset,
+ drm_intel_bo *target_bo, uint32_t
target_offset,
+ uint32_t read_domains, uint32_t write_domain);
int drm_intel_bo_emit_reloc_fence(drm_intel_bo *bo, uint32_t offset,
drm_intel_bo *target_bo,
uint32_t target_offset,
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 41de396..713ed4e 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -140,6 +140,7 @@ typedef struct _drm_intel_bufmgr_gem {
} drm_intel_bufmgr_gem;
#define DRM_INTEL_RELOC_FENCE (1<<0)
+#define DRM_INTEL_RELOC_SUPPORTS_48B_ADDRESS (2<<0)
typedef struct _drm_intel_reloc_target_info {
drm_intel_bo *bo;
@@ -237,6 +238,14 @@ struct _drm_intel_bo_gem {
bool is_userptr;
/**
+* Boolean of whether this buffer can be
[Intel-gfx] [PATCH libdrm v3 0/2] 48-bit virtual address support in i915
48-bit virtual address range will be enabled in i915 soon, but some objects must be referenced by 32-bit offsets. These patches use a new kernel flag to specify if this restriction applies or not. I'm sending these patches to comply with the i915 merge process. Once the kernel patch is merged, I'll make a new libdrm release and address the mesa build dependency. Michel Thierry (2): intel: Add EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag intel: add new function name to symbol-check test include/drm/i915_drm.h| 3 ++- intel/intel-symbol-check | 1 + intel/intel_bufmgr.c | 16 ++ intel/intel_bufmgr.h | 6 +- intel/intel_bufmgr_gem.c | 54 +++ intel/intel_bufmgr_priv.h | 11 ++ 6 files changed, 85 insertions(+), 6 deletions(-) -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH mesa v3] i965/gen8+: bo in state base address must be in 32-bit address range
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.
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/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -409,11 +409,23 @@ bool
intel_batchbuffer_emit_reloc64(struct brw_context *brw,
drm_intel_bo *buffer,
uint32_t read_domains, uint32_t write_domain,
- uint32_t delta)
+ uint32_t delta,
+ bool support_48bit_offset)
{
- int ret = drm_intel_bo_emit_reloc(brw->batch.bo, 4*brw->batch.used,
- buffer, delta,
- read_domains, write_domain);
+ int ret;
+
+ /* Not all buffers can be allocated outside the first 4GB, and
+* offset must be limited to 32-bits.
+*/
+ if (support_48bit_offset)
+ drm_intel_bo_emit_reloc_48bit(brw->batch.bo, 4*brw->batch.used,
+buffer, delta,
+read_domains, write_domain);
+ else
+ drm_intel_bo_emit_reloc(brw->batch.bo, 4*brw->batch.used,
+ buffer, delta,
+ read_domains, write_domain);
+
assert(ret == 0);
(void) ret;
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
b/src/
[Intel-gfx] [PATCH libdrm v3 2/2] intel: add new function name to symbol-check test
Signed-off-by: Michel Thierry --- intel/intel-symbol-check | 1 + 1 file changed, 1 insertion(+) diff --git a/intel/intel-symbol-check b/intel/intel-symbol-check index c555e6d..6f8450b 100755 --- a/intel/intel-symbol-check +++ b/intel/intel-symbol-check @@ -18,6 +18,7 @@ drm_intel_bo_busy drm_intel_bo_disable_reuse drm_intel_bo_emit_reloc drm_intel_bo_emit_reloc_fence +drm_intel_bo_emit_reloc_48bit drm_intel_bo_exec drm_intel_bo_fake_alloc_static drm_intel_bo_fake_disable_backing_store -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: fix checksum write for automated test reply
From: "Thulasimani,Sivakumar"
DP spec requires the checksum of the last block read to be written
when replying to TEST_EDID_READ. This patch fixes the current code
to do the same.
v2: removed loop for jumping blocks and performed direct addition
as recommended by Daniel
Signed-off-by: Sivakumar Thulasimani
---
drivers/gpu/drm/i915/intel_dp.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f1b9f93..fa6e202 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4090,9 +4090,16 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp
*intel_dp)
intel_dp->aux.i2c_defer_count);
intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
} else {
+ struct edid *block = intel_connector->detect_edid;
+
+ /* We have to write the checksum
+* of the last block read
+*/
+ block += intel_connector->detect_edid->extensions;
+
if (!drm_dp_dpcd_write(&intel_dp->aux,
DP_TEST_EDID_CHECKSUM,
- &intel_connector->detect_edid->checksum,
+ &block->checksum,
1))
DRM_DEBUG_KMS("Failed to write EDID checksum\n");
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer
On 8/6/2015 11:00 PM, Daniel Vetter wrote: On Thu, Aug 6, 2015 at 10:17 PM, kbuild test robot wrote: 1070 if (IS_ENABLED(CONFIG_X86_32)) 1071 /* While we have a proliferation of size_t variables 1072 * we cannot represent the full ppgtt size on 32bit, 1073 * so limit it to the same size as the GGTT (currently 1074 * 2GiB). 1075 */ 1076 ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total; 1077 ppgtt->base.cleanup = gen8_ppgtt_cleanup; 1078 ppgtt->base.allocate_va_range = gen8_alloc_va_range; 1079 ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; 1080 ppgtt->base.clear_range = gen8_ppgtt_clear_range; 1081 ppgtt->base.unbind_vma = ppgtt_unbind_vma; 1082 ppgtt->base.bind_vma = ppgtt_bind_vma; 1083 1084 ppgtt->switch_mm = gen8_mm_switch; 1085 1086 ret = __pdp_init(false, &ppgtt->pdp); So the first argument of pdp_init ist struct drm_device *dev and yes the first thing it does is deref it. *dev is used only for I915_PDPES_PER_PDP/USES_FULL_48BIT_PPGTT, which in this path is always false. I didn't expect kbuild to complain. I'll change it with the other modifications I'm about to send. How exactly was this tested again? Oh and the hunk right below with the CONFIG_X86_32 needs to got too - if we're 48b safe then we should be 32b safe too ;-) Yes, after the offset change it'll be completely safe. I left it in this patch as that was only moving code around. I'll include a proper "revert-me" patch, saying why it is safe now. -Michel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Check idle to active before processing CSQ
Daniel Vetter writes:
> On Thu, Aug 06, 2015 at 05:09:17PM +0300, Mika Kuoppala wrote:
>> If idle to active bit is set, the rest of the fields
>> in CSQ are not valid.
>>
>> Bail out early if this is the case in order to prevent
>> rest of the loop inspecting stale values.
>>
>> Signed-off-by: Mika Kuoppala
>
> Same questions here too, what's the impact. E.g. if you only found this by
> bspec/code inspection then it's for -next, but if it's to fix some known
> breakage then it's for -fixes + cc: stable.
>
To this and the masked write one: Both of these were found
when I was trying to find out root cause for skl hangs.
They are both for -next. Both are in the correctness
department vrt bspec and I haven't observed any other
impact.
Point taken on being more verbose.
-Mika
> Thanks, Daniel
>
>> ---
>> drivers/gpu/drm/i915/intel_lrc.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
>> b/drivers/gpu/drm/i915/intel_lrc.c
>> index 99bba8e..96218bf 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -497,6 +497,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>> status_id = I915_READ(RING_CONTEXT_STATUS_BUF(ring) +
>> (read_pointer % 6) * 8 + 4);
>>
>> +if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
>> +continue;
>> +
>> if (status & GEN8_CTX_STATUS_PREEMPTED) {
>> if (status & GEN8_CTX_STATUS_LITE_RESTORE) {
>> if (execlists_check_remove_request(ring,
>> status_id))
>> --
>> 2.1.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
Re: [Intel-gfx] [PATCH] drm/i915: Only move to the CPU write domain if keeping the GTT pages
On Thu, Aug 06, 2015 at 05:43:39PM +0100, Chris Wilson wrote:
> We have for a long time been ultra-paranoid about the situation whereby
> we hand back pages to the system that have been written to by the GPU
> and potentially simultaneously by the user through a CPU mmapping. We
> can relax this restriction when we know that the cache domain tracking
> is true and there can be no stale cacheline invalidatations. This is
> true if the object has never been CPU mmaped as all internal accesses
> (i.e. kmap/iomap) are carefully flushed. For a CPU mmaping, one would
> expect that the invalid cache lines are resolved on PTE/TLB shootdown
> during munmap(), so the only situation we need to be paranoid about is
That seems a pretty strong assumption given that x86 cache are physically
indexed - I'd never expect flushing to happen on munmap.
> when such a CPU mmaping exists at the time of put_pages. Given that we
> need to treat put_pages carefully as we may return live data to the
> system that we want to use again in the future (i.e. I915_MADV_WILLNEED
> pages) we can simply treat a live CPU mmaping as a special case of
> WILLNEED (which it is!). Any I915_MADV_DONTNEED pages and their
> mmapings are shotdown immediately following put_pages.
>
> Signed-off-by: Chris Wilson
> Cc: "Goel, Akash"
> Cc: Ville Syrjälä
> Cc: Daniel Vetter
> Cc: Jesse Barnes
But it's still salvageable I think since we only care about coherency for
the gpu (where data might be stuck in cpu caches). From the cpu's pov (and
hence the entire system except the gpu) we should never see inconsistency
really - as soon as the gpu does a write to a cacheline it'll win, and
before that nothing in the system can assume anything about the contents
of these pages.
So imo a simple
/* For purgeable objects we don't care about object contents. */
would be enough.
Well except that there's gl extensions which expose purgeable objects, so
I guess we can't actually do this.
I presume though you only want to avoid clflush when actually purging an
object, so maybe we can keep this by purging the shmem backing node first
and checking here for __I915_MADV_PURGED instead?
Oh and some perf data would be good for this.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 49
> ++---
> 1 file changed, 36 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 2dfe707f11d3..24deace364a5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2047,22 +2047,45 @@ i915_gem_object_put_pages_gtt(struct
> drm_i915_gem_object *obj)
>
> BUG_ON(obj->madv == __I915_MADV_PURGED);
>
> - ret = i915_gem_object_set_to_cpu_domain(obj, true);
> - if (ret) {
> - /* In the event of a disaster, abandon all caches and
> - * hope for the best.
> - */
> - WARN_ON(ret != -EIO);
> - i915_gem_clflush_object(obj, true);
> - obj->base.read_domains = obj->base.write_domain =
> I915_GEM_DOMAIN_CPU;
> - }
> -
> i915_gem_gtt_finish_object(obj);
>
> - if (i915_gem_object_needs_bit17_swizzle(obj))
> - i915_gem_object_save_bit_17_swizzle(obj);
> + /* If we need to access the data in the future, we need to
> + * be sure that the contents of the object is coherent with
> + * the CPU prior to releasing the pages back to the system.
> + * Once we unpin them, the mm is free to move them to different
> + * zones or even swap them out to disk - all without our
> + * intervention. (Though we could track such operations with our
> + * own gemfs, if we ever write one.) As such if we want to keep
> + * the data, set it to the CPU domain now just in case someone
> + * else touches it.
> + *
> + * For a long time we have been paranoid about handing back
> + * pages to the system with stale cacheline invalidation. For
> + * all internal use (kmap/iomap), we know that the domain tracking is
> + * accurate. However, the userspace API is lax and the user can CPU
> + * mmap the object and invalidate cachelines without our accurate
> + * tracking. We have been paranoid to be sure that we always flushed
> + * the cachelines when we stopped using the pages. However, given
> + * that the CPU PTE/TLB shootdown must have invalidated the cachelines
> + * upon munmap(), we only need to be paranoid about a live CPU mmap
> + * now. For this, we need only treat it as live data see
> + * discard_backing_storage().
> + */
> + if (obj->madv == I915_MADV_WILLNEED) {
> + ret = i915_gem_object_set_to_cpu_domain(obj, true);
> + if (ret) {
> + /* In the event of a disaster, abandon all caches and
> + * hope for the best.
> + */
> + WARN_ON(ret !
Re: [Intel-gfx] [PATCH v6 17/19] drm/i915: Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset
On Thu, Aug 06, 2015 at 05:27:38PM +0100, Michel Thierry wrote: > On 8/6/2015 1:47 PM, Daniel Vetter wrote: > >On Wed, Aug 05, 2015 at 05:14:33PM +0100, Michel Thierry wrote: > >>On 8/5/2015 4:58 PM, Daniel Vetter wrote: > >>>On Wed, Jul 29, 2015 at 05:24:01PM +0100, Michel Thierry wrote: > There are some allocations that must be only referenced by 32-bit > offsets. To limit the chances of having the first 4GB already full, > objects not requiring this workaround use DRM_MM_SEARCH_BELOW/ > DRM_MM_CREATE_TOP flags > > In specific, any resource used with flat/heapless (0x-0xf000) > General State Heap (GSH) or Instruction State Heap (ISH) must be in a > 32-bit range, because the General State Offset and Instruction State > Offset are limited to 32-bits. > > Objects must have EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag to indicate if > they can be allocated above the 32-bit address range. To limit the > chances of having the first 4GB already full, objects will use > DRM_MM_SEARCH_BELOW + DRM_MM_CREATE_TOP flags when possible. > > v2: Changed flag logic from neeeds_32b, to supports_48b. > v3: Moved 48-bit support flag back to exec_object. (Chris, Daniel) > v4: Split pin flags into PIN_ZONE_4G and PIN_HIGH; update PIN_OFFSET_MASK > to use last PIN_ defined instead of hard-coded value; use correct limit > check in eb_vma_misplaced. (Chris) > v5: Don't touch PIN_OFFSET_MASK and update workaround comment (Chris) > v6: Apply pin-high for ggtt too (Chris) > v7: Handle simultaneous pin-high and pin-mappable end correctly (Akash) > Fix check for entries currently using +4GB addresses, use min_t and > other polish in object_bind_to_vm (Chris) > > Cc: Chris Wilson > Cc: Akash Goel > Reviewed-by: Chris Wilson (v4) > Signed-off-by: Michel Thierry > >>> > >>>For the record, where can I find the mesa patches for this? I think for > >>>simple things like this a References: line point to the relevant UMD > >>>patches in mailing-list archives would be great. > >>>-Daniel > >>> > >> > >>Here they are, > >> > >>References: > >>http://lists.freedesktop.org/archives/dri-devel/2015-July/085501.html and > >>http://lists.freedesktop.org/archives/mesa-dev/2015-July/088003.html > > > >Sounds like there's still another revision we need to do on those? > > Yes, a couple of changes, set/clear functions internal in libdrm and update > the symbol-check test. > > I put it on hold, because I was also asked to not include the libdrm changes > until the updated kernel header (EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag) was > merged. > > Then I also need to create a libdrm release, and update mesa's dependency to > this new version number. Nope, we need everything before I can pull in the kernel patch. Once that happens then you can do the release/depency dance (of course don't include those bits in your proposed patches yet). -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] Intel-kms in Linux-4.2rc causes regression due to dithering always on.
On Fri, Aug 07, 2015 at 12:45:52AM +0200, Mario Kleiner wrote: > On 08/07/2015 12:12 AM, Daniel Vetter wrote: > >On Thu, Aug 6, 2015 at 11:56 PM, Mario Kleiner > > wrote: > >>Hi Daniel and all, > >> > >>since Linux 4.2 (tested with rc4), i think this commit > >>d328c9d78d64ca11e744fe227096990430a88477 > >>"drm/i915: Select starting pipe bpp irrespective or the primary plane" > >> > >>causes trouble for me and my users, as tested on Intel HD Ironlake and Ivy > >>Bridge with MiniDP->Singlelink-DVI adapter -> Measurement device. > >> > >>Afaics it causes dithering to always be enabled on a regular 8bpc > >>framebuffer, even when outputting to a 8 bpc DVI-D output, and that > >>dithering causes my display measurement equipment and other special display > >>devices used for neuro-science and medical applications to fail. This > >>equipment requires an identity passthrough of 8 bpc framebuffer pixels to > >>the digital outputs, iow. dithering off. > >> > >>Log output on Linux 4.1 (good): > >> > >>Aug 1 06:39:26 twisty kernel: [ 154.175394] > >>[drm:connected_sink_compute_bpp] [CONNECTOR:35:HDMI-A-1] checking for sink > >>bpp constrains > >>Aug 1 06:39:26 twisty kernel: [ 154.175396] > >>[drm:intel_hdmi_compute_config] picking bpc to 8 for HDMI output > >>Aug 1 06:39:26 twisty kernel: [ 154.175397] > >>[drm:intel_hdmi_compute_config] forcing pipe bpc to 24 for HDMI > >>Aug 1 06:39:26 twisty kernel: [ 154.175400] [drm:ironlake_check_fdi_lanes] > >>checking fdi config on pipe A, lanes 1 > >>Aug 1 06:39:26 twisty kernel: [ 154.175402] > >>[drm:intel_modeset_pipe_config] plane bpp: 24, pipe bpp: 24, dithering: 0 > >>Aug 1 06:39:26 twisty kernel: [ 154.175403] [drm:intel_dump_pipe_config] > >>[CRTC:20][modeset] config for pipe A > >>Aug 1 06:39:26 twisty kernel: [ 154.175404] [drm:intel_dump_pipe_config] > >>cpu_transcoder: A > >>Aug 1 06:39:26 twisty kernel: [ 154.175405] [drm:intel_dump_pipe_config] > >>pipe bpp: 24, dithering: 0 > >> > >>Log output on Linux 4.2-rc4 (bad): > >> > >>Aug 1 06:21:31 twisty kernel: [ 200.924831] > >>[drm:connected_sink_compute_bpp] [CONNECTOR:36:HDMI-A-1] checking for sink > >>bpp constrains > >>Aug 1 06:21:31 twisty kernel: [ 200.924832] > >>[drm:connected_sink_compute_bpp] clamping display bpp (was 36) to default > >>limit of 24 > >>Aug 1 06:21:31 twisty kernel: [ 200.924834] > >>[drm:intel_hdmi_compute_config] picking bpc to 8 for HDMI output > >>Aug 1 06:21:31 twisty kernel: [ 200.924835] > >>[drm:intel_hdmi_compute_config] forcing pipe bpc to 24 for HDMI > >>Aug 1 06:21:31 twisty kernel: [ 200.924838] [drm:ironlake_check_fdi_lanes] > >>checking fdi config on pipe A, lanes 1 > >>Aug 1 06:21:31 twisty kernel: [ 200.924840] > >>[drm:intel_modeset_pipe_config] plane bpp: 36, pipe bpp: 24, dithering: 1 > >>Aug 1 06:21:31 twisty kernel: [ 200.924841] [drm:intel_dump_pipe_config] > >>[CRTC:21][modeset] config 880131a5c800 for pipe A > >>Aug 1 06:21:31 twisty kernel: [ 200.924842] [drm:intel_dump_pipe_config] > >>cpu_transcoder: A > >>Aug 1 06:21:31 twisty kernel: [ 200.924843] [drm:intel_dump_pipe_config] > >>pipe bpp: 24, dithering: 1 > >> > >>Ideas what to do about this? > > > >Well I somehow assumed the dither bit would be sane and not wreak > >havoc with the lower bits when they would fit into the final bpc pipe > >mode ... Can you confirm with your equipment that we seem to be doing > >8bpc->6bpc dithering on the 8bpc sink? > > > > It will need a bit of work to find this out when i'm back in the lab. So far > i just know something bad is happening to the signal and i assume it's the > dithering, because the visual error pattern of messiness looks like that > caused by dithering. E.g., on a static framebuffer i see some repeating > pattern over the screen, but the pattern changes with every OpenGL > bufferswap, even if i swap to the same fb content, as if the swap triggers > some change of the spatial dither pattern (assuming PIPECONF_DITHER_TYPE_SP > = spatial dithering?) > > >If that's the case we simply limit to only ever dither when the sink > >is 6bpc, and not in any other case. > >-Daniel > > > > That would be an improvement for my immediate problem if that works. But > assuming we have 10 bpc framebuffers at some point, dithering 10 bpc -> 8 > bpc would also have some practical use. > > Probably some dynamic check would be good, a la if there is a mismatch > between the max(bpc) over all active planes and the supported depth of the > sink then dither? > > It's not clear to me where the dithering happens on intel hw. I'd expected > that with a 24 bpp framebuffer feeding into a 24 bpp pipe, dithering simply > wouldn't do anything even if enabled. Yeah my assumption was that if you run the pipe at a given bpc it will just pass through anything that fits and only dither the additional bits. But obviously that's not how the hardware works ... The problem with adaptive schemes is that we have multiple planes nowadays and they might all run
Re: [Intel-gfx] Enable PSR in IvyBridge?
On Fri, 7 Aug, 2015 at 12:56 AM, Rodrigo Vivi wrote: On Thu, Aug 6, 2015 at 1:20 PM harrykipper wrote: Hello, I just discovered that Intel introduced PSR with IvyBridge, so I tried Unfortunately this information is incorrect. There is no PSR on IVB. No single mention in the specs here. Right. Sorry. I was misled by this Register article http://www.theregister.co.uk/2011/09/14/intel_demos_panel_self_refresh_display_tech/ It explicitly says "Intel will be supporting the tech in its 'Ivy Bridge' processors" :-( I even had a feeling that after "activating" it the laptop was drawing less power :-D ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
