Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1506063 > unrolled thread
| Started by | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| First post | 2016-10-21 17:30 +0200 |
| Last post | 2016-10-22 07:20 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 1/2] drm/i915/gvt: add ACPI and 64BIT dependencies Arnd Bergmann <arnd@arndb.de> - 2016-10-21 17:30 +0200
[PATCH 2/2] drm/i915/gvt: fix compilation Arnd Bergmann <arnd@arndb.de> - 2016-10-21 17:30 +0200
Re: [PATCH 2/2] drm/i915/gvt: fix compilation Zhenyu Wang <zhenyuw@linux.intel.com> - 2016-10-22 07:20 +0200
Re: [PATCH 1/2] drm/i915/gvt: add ACPI and 64BIT dependencies Zhenyu Wang <zhenyuw@linux.intel.com> - 2016-10-22 07:20 +0200
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-10-21 17:30 +0200 |
| Subject | [PATCH 1/2] drm/i915/gvt: add ACPI and 64BIT dependencies |
| Message-ID | <suJVD-7tM-9@gated-at.bofh.it> |
The newly added gvt code produces lots of serious warnings and errors
when either built on 32-bit x86, or built with ACPI disabled, e.g.
drivers/gpu/drm/i915/gvt/gtt.c: In function ‘read_pte64’:
drivers/gpu/drm/i915/gvt/gtt.c:277:2: error: left shift count >= width of type [-Werror]
drivers/gpu/drm/i915/gvt/gtt.c: In function ‘gen8_gtt_get_pfn’:
drivers/gpu/drm/i915/gvt/gtt.c:360:3: error: left shift count >= width of type [-Werror]
drivers/gpu/drm/i915/gvt/opregion.c: In function ‘intel_gvt_init_opregion’:
drivers/gpu/drm/i915/gvt/opregion.c:183:2: error: implicit declaration of function ‘acpi_os_ioremap’ [-Werror=implicit-function-declaration]
This avoids the problems by simply disallowing those configurations
in Kconfig. I'm sure it's possible to make the code more portable
and support building GVT without those options, but it might not be
useful to do so.
Fixes: 4d60c5fd3f87 ("drm/i915/gvt: vGPU PCI configuration space virtualization")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
If the code is meant to work on 32-bit and non-ACPI kernels, please
treat this as a bug report and disregard the patch.
---
drivers/gpu/drm/i915/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 6d4194288d11..1b9308284dde 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -84,6 +84,7 @@ config DRM_I915_USERPTR
config DRM_I915_GVT
bool "Enable Intel GVT-g graphics virtualization host support"
depends on DRM_I915
+ depends on 64BIT && ACPI
default n
help
Choose this option if you want to enable Intel GVT-g graphics
--
2.9.0
[toc] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-10-21 17:30 +0200 |
| Subject | [PATCH 2/2] drm/i915/gvt: fix compilation |
| Message-ID | <suJVE-7tM-21@gated-at.bofh.it> |
| In reply to | #1506063 |
Two functions in the newly added gvt render code are obviously
broken, as they reference a variable without initialization and
don't reference another variable at all:
drivers/gpu/drm/i915/gvt/render.c: In function ‘intel_gvt_load_render_mmio’:
drivers/gpu/drm/i915/gvt/render.c:148:13: error: ‘offset.reg’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/gpu/drm/i915/gvt/render.c: In function ‘intel_gvt_restore_render_mmio’:
drivers/gpu/drm/i915/gvt/render.c:185:13: error: ‘offset.reg’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
This is probably not a correct fix, but it gets us a clean build
by removing the unused arrays and initializing the offset variable
to something that potentially might be correct.
Fixes: 178657139307 ("drm/i915/gvt: vGPU context switch")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/gpu/drm/i915/gvt/render.c | 25 +++----------------------
1 file changed, 3 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
index feebb65ba641..79e112288065 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -147,29 +147,20 @@ static void load_mocs(struct intel_vgpu *vgpu, int ring_id)
{
struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
i915_reg_t offset, l3_offset;
- u32 regs[] = {
- [RCS] = 0xc800,
- [VCS] = 0xc900,
- [VCS2] = 0xca00,
- [BCS] = 0xcc00,
- [VECS] = 0xcb00,
- };
int i;
- if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
- return;
-
if (!IS_SKYLAKE(dev_priv))
return;
for (i = 0; i < 64; i++) {
+ offset.reg = i * 4;
gen9_render_mocs[ring_id][i] = I915_READ(offset);
I915_WRITE(offset, vgpu_vreg(vgpu, offset));
POSTING_READ(offset);
- offset.reg += 4;
}
if (ring_id == RCS) {
+ offset.reg = 64 * 4;
l3_offset.reg = 0xb020;
for (i = 0; i < 32; i++) {
gen9_render_mocs_L3[i] = I915_READ(l3_offset);
@@ -184,26 +175,16 @@ static void restore_mocs(struct intel_vgpu *vgpu, int ring_id)
{
struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
i915_reg_t offset, l3_offset;
- u32 regs[] = {
- [RCS] = 0xc800,
- [VCS] = 0xc900,
- [VCS2] = 0xca00,
- [BCS] = 0xcc00,
- [VECS] = 0xcb00,
- };
int i;
- if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
- return;
-
if (!IS_SKYLAKE(dev_priv))
return;
for (i = 0; i < 64; i++) {
+ offset.reg = i * 4;
vgpu_vreg(vgpu, offset) = I915_READ(offset);
I915_WRITE(offset, gen9_render_mocs[ring_id][i]);
POSTING_READ(offset);
- offset.reg += 4;
}
if (ring_id == RCS) {
--
2.9.0
[toc] | [prev] | [next] | [standalone]
| From | Zhenyu Wang <zhenyuw@linux.intel.com> |
|---|---|
| Date | 2016-10-22 07:20 +0200 |
| Subject | Re: [PATCH 2/2] drm/i915/gvt: fix compilation |
| Message-ID | <suWSR-7yB-3@gated-at.bofh.it> |
| In reply to | #1506065 |
[Multipart message — attachments visible in raw view] — view raw
On 2016.10.21 17:25:50 +0200, Arnd Bergmann wrote:
> Two functions in the newly added gvt render code are obviously
> broken, as they reference a variable without initialization and
> don't reference another variable at all:
>
> drivers/gpu/drm/i915/gvt/render.c: In function ???intel_gvt_load_render_mmio???:
> drivers/gpu/drm/i915/gvt/render.c:148:13: error: ???offset.reg??? may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/gpu/drm/i915/gvt/render.c: In function ???intel_gvt_restore_render_mmio???:
> drivers/gpu/drm/i915/gvt/render.c:185:13: error: ???offset.reg??? may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This is probably not a correct fix, but it gets us a clean build
> by removing the unused arrays and initializing the offset variable
> to something that potentially might be correct.
>
> Fixes: 178657139307 ("drm/i915/gvt: vGPU context switch")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
I think the correct fix is like
diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
index feebb65..cc23c3f 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -162,6 +162,7 @@ static void load_mocs(struct intel_vgpu *vgpu, int ring_id)
if (!IS_SKYLAKE(dev_priv))
return;
+ offset.reg = regs[ring_id];
for (i = 0; i < 64; i++) {
gen9_render_mocs[ring_id][i] = I915_READ(offset);
I915_WRITE(offset, vgpu_vreg(vgpu, offset));
@@ -199,6 +200,7 @@ static void restore_mocs(struct intel_vgpu *vgpu, int ring_id)
if (!IS_SKYLAKE(dev_priv))
return;
+ offset.reg = regs[ring_id];
for (i = 0; i < 64; i++) {
vgpu_vreg(vgpu, offset) = I915_READ(offset);
I915_WRITE(offset, gen9_render_mocs[ring_id][i]);
Thanks for pointing this out, it's a mistake during our code preparation for upstream.
I'll queue this up.
> drivers/gpu/drm/i915/gvt/render.c | 25 +++----------------------
> 1 file changed, 3 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
> index feebb65ba641..79e112288065 100644
> --- a/drivers/gpu/drm/i915/gvt/render.c
> +++ b/drivers/gpu/drm/i915/gvt/render.c
> @@ -147,29 +147,20 @@ static void load_mocs(struct intel_vgpu *vgpu, int ring_id)
> {
> struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> i915_reg_t offset, l3_offset;
> - u32 regs[] = {
> - [RCS] = 0xc800,
> - [VCS] = 0xc900,
> - [VCS2] = 0xca00,
> - [BCS] = 0xcc00,
> - [VECS] = 0xcb00,
> - };
> int i;
>
> - if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
> - return;
> -
> if (!IS_SKYLAKE(dev_priv))
> return;
>
> for (i = 0; i < 64; i++) {
> + offset.reg = i * 4;
> gen9_render_mocs[ring_id][i] = I915_READ(offset);
> I915_WRITE(offset, vgpu_vreg(vgpu, offset));
> POSTING_READ(offset);
> - offset.reg += 4;
> }
>
> if (ring_id == RCS) {
> + offset.reg = 64 * 4;
> l3_offset.reg = 0xb020;
> for (i = 0; i < 32; i++) {
> gen9_render_mocs_L3[i] = I915_READ(l3_offset);
> @@ -184,26 +175,16 @@ static void restore_mocs(struct intel_vgpu *vgpu, int ring_id)
> {
> struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> i915_reg_t offset, l3_offset;
> - u32 regs[] = {
> - [RCS] = 0xc800,
> - [VCS] = 0xc900,
> - [VCS2] = 0xca00,
> - [BCS] = 0xcc00,
> - [VECS] = 0xcb00,
> - };
> int i;
>
> - if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
> - return;
> -
> if (!IS_SKYLAKE(dev_priv))
> return;
>
> for (i = 0; i < 64; i++) {
> + offset.reg = i * 4;
> vgpu_vreg(vgpu, offset) = I915_READ(offset);
> I915_WRITE(offset, gen9_render_mocs[ring_id][i]);
> POSTING_READ(offset);
> - offset.reg += 4;
> }
>
> if (ring_id == RCS) {
> --
> 2.9.0
>
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
[toc] | [prev] | [next] | [standalone]
| From | Zhenyu Wang <zhenyuw@linux.intel.com> |
|---|---|
| Date | 2016-10-22 07:20 +0200 |
| Message-ID | <suWSR-7yB-7@gated-at.bofh.it> |
| In reply to | #1506063 |
[Multipart message — attachments visible in raw view] — view raw
On 2016.10.21 17:25:49 +0200, Arnd Bergmann wrote:
> The newly added gvt code produces lots of serious warnings and errors
> when either built on 32-bit x86, or built with ACPI disabled, e.g.
>
> drivers/gpu/drm/i915/gvt/gtt.c: In function ???read_pte64???:
> drivers/gpu/drm/i915/gvt/gtt.c:277:2: error: left shift count >= width of type [-Werror]
> drivers/gpu/drm/i915/gvt/gtt.c: In function ???gen8_gtt_get_pfn???:
> drivers/gpu/drm/i915/gvt/gtt.c:360:3: error: left shift count >= width of type [-Werror]
> drivers/gpu/drm/i915/gvt/opregion.c: In function ???intel_gvt_init_opregion???:
> drivers/gpu/drm/i915/gvt/opregion.c:183:2: error: implicit declaration of function ???acpi_os_ioremap??? [-Werror=implicit-function-declaration]
>
> This avoids the problems by simply disallowing those configurations
> in Kconfig. I'm sure it's possible to make the code more portable
> and support building GVT without those options, but it might not be
> useful to do so.
>
> Fixes: 4d60c5fd3f87 ("drm/i915/gvt: vGPU PCI configuration space virtualization")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> If the code is meant to work on 32-bit and non-ACPI kernels, please
> treat this as a bug report and disregard the patch.
> ---
Thanks, Arnd. We have to depend on 64bit now and not require for ACPI,
as we used one acpi function for opregion mem map which is not necessary,
so I queued one 64bit dependence and another to remove acpi dependence for Daniel.
> drivers/gpu/drm/i915/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 6d4194288d11..1b9308284dde 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -84,6 +84,7 @@ config DRM_I915_USERPTR
> config DRM_I915_GVT
> bool "Enable Intel GVT-g graphics virtualization host support"
> depends on DRM_I915
> + depends on 64BIT && ACPI
> default n
> help
> Choose this option if you want to enable Intel GVT-g graphics
> --
> 2.9.0
>
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web