[Mesa-dev] [PATCH 1/5] swr/rast: Remove draw type from event definitions

[George Kyriazis]

- Have the draw type sent to DrawInfoEvent in handlers created in
  archrast.cpp.  The draw type no longer needs to be sent during during
  AR_API_EVENT() call in api.cpp.

- Remove draw type from event defintions in events_private.proto, no
  longer needed
---
 src/gallium/drivers/swr/rasterizer/archrast/archrast.cpp | 8 
 src/gallium/drivers/swr/rasterizer/archrast/events_private.proto | 4 
 src/gallium/drivers/swr/rasterizer/core/api.cpp  | 8 
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/src/gallium/drivers/swr/rasterizer/archrast/archrast.cpp 
b/src/gallium/drivers/swr/rasterizer/archrast/archrast.cpp
index d7a3b29..8c09411 100644
--- a/src/gallium/drivers/swr/rasterizer/archrast/archrast.cpp
+++ b/src/gallium/drivers/swr/rasterizer/archrast/archrast.cpp
@@ -175,28 +175,28 @@ namespace ArchRast
 
 virtual void Handle(const DrawInstancedEvent& event)
 {
-DrawInfoEvent e(event.data.drawId, event.data.type, 
event.data.topology, event.data.numVertices, 0, 0, event.data.startVertex, 
event.data.numInstances, event.data.startInstance);
+DrawInfoEvent e(event.data.drawId, ArchRast::Instanced, 
event.data.topology, event.data.numVertices, 0, 0, event.data.startVertex, 
event.data.numInstances, event.data.startInstance);
 
 EventHandlerFile::Handle(e);
 }
 
 virtual void Handle(const DrawIndexedInstancedEvent& event)
 {
-DrawInfoEvent e(event.data.drawId, event.data.type, 
event.data.topology, 0, event.data.numIndices, event.data.indexOffset, 
event.data.baseVertex, event.data.numInstances, event.data.startInstance);
+DrawInfoEvent e(event.data.drawId, ArchRast::IndexedInstanced, 
event.data.topology, 0, event.data.numIndices, event.data.indexOffset, 
event.data.baseVertex, event.data.numInstances, event.data.startInstance);
 
 EventHandlerFile::Handle(e);
 }
 
 virtual void Handle(const DrawInstancedSplitEvent& event)
 {
-DrawInfoEvent e(event.data.drawId, event.data.type, 0, 0, 0, 0, 0, 
0, 0);
+DrawInfoEvent e(event.data.drawId, ArchRast::InstancedSplit, 0, 0, 
0, 0, 0, 0, 0);
 
 EventHandlerFile::Handle(e);
 }
 
 virtual void Handle(const DrawIndexedInstancedSplitEvent& event)
 {
-DrawInfoEvent e(event.data.drawId, event.data.type, 0, 0, 0, 0, 0, 
0, 0);
+DrawInfoEvent e(event.data.drawId, 
ArchRast::IndexedInstancedSplit, 0, 0, 0, 0, 0, 0, 0);
 
 EventHandlerFile::Handle(e);
 }
diff --git a/src/gallium/drivers/swr/rasterizer/archrast/events_private.proto 
b/src/gallium/drivers/swr/rasterizer/archrast/events_private.proto
index 71b723d..8970141 100644
--- a/src/gallium/drivers/swr/rasterizer/archrast/events_private.proto
+++ b/src/gallium/drivers/swr/rasterizer/archrast/events_private.proto
@@ -117,7 +117,6 @@ event ClipInfoEvent
 event DrawInstancedEvent
 {
 uint32_t drawId;
-AR_DRAW_TYPE type;
 uint32_t topology;
 uint32_t numVertices;
 int32_t  startVertex;
@@ -128,7 +127,6 @@ event DrawInstancedEvent
 event DrawIndexedInstancedEvent
 {
 uint32_t drawId;
-AR_DRAW_TYPE type;
 uint32_t topology;
 uint32_t numIndices;
 int32_t  indexOffset;
@@ -141,12 +139,10 @@ event DrawIndexedInstancedEvent
 event DrawInstancedSplitEvent
 {
 uint32_t drawId;
-AR_DRAW_TYPE type;
 };
 
 ///@brief API Stat: Split draw event for DrawIndexedInstanced.
 event DrawIndexedInstancedSplitEvent
 {
 uint32_t drawId;
-AR_DRAW_TYPE type;
 };
diff --git a/src/gallium/drivers/swr/rasterizer/core/api.cpp 
b/src/gallium/drivers/swr/rasterizer/core/api.cpp
index cb98cbe..99d3cd5 100644
--- a/src/gallium/drivers/swr/rasterizer/core/api.cpp
+++ b/src/gallium/drivers/swr/rasterizer/core/api.cpp
@@ -1169,7 +1169,7 @@ void DrawInstanced(
 DRAW_CONTEXT* pDC = GetDrawContext(pContext);
 
 RDTSC_BEGIN(APIDraw, pDC->drawId);
-AR_API_EVENT(DrawInstancedEvent(pDC->drawId, ArchRast::Instanced, 
topology, numVertices, startVertex, numInstances, startInstance));
+AR_API_EVENT(DrawInstancedEvent(pDC->drawId, topology, numVertices, 
startVertex, numInstances, startInstance));
 
 uint32_t maxVertsPerDraw = MaxVertsPerDraw(pDC, numVertices, topology);
 uint32_t primsPerDraw = GetNumPrims(topology, maxVertsPerDraw);
@@ -1221,7 +1221,7 @@ void DrawInstanced(
 //enqueue DC
 QueueDraw(pContext);
 
-AR_API_EVENT(DrawInstancedSplitEvent(pDC->drawId, 
ArchRast::InstancedSplit));
+AR_API_EVENT(DrawInstancedSplitEvent(pDC->drawId));
 
 remainingVerts -= numVertsForDraw;
 draw++;
@@ -1297,7 +1297,7 @@ void DrawIndexedInstance(
 API_STATE* pState = &pDC->pState->state;
 
 RDTSC_BEGIN(APIDrawIndexed, pDC->drawId);
-AR_API_EVENT(DrawIndexedInstancedEvent(pDC->drawId, 
ArchRast::IndexedInstancedSplit, topology, numIndices, indexOffset

[Mesa-dev] [PATCH 5/5] swr/rast: revert clip distance precision

[George Kyriazis]

Breaks a whole bunch of piglit tests
---
 src/gallium/drivers/swr/rasterizer/core/backend_impl.h |  4 +---
 src/gallium/drivers/swr/rasterizer/core/binner.cpp | 17 -
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/swr/rasterizer/core/backend_impl.h 
b/src/gallium/drivers/swr/rasterizer/core/backend_impl.h
index 454f473..2cfd52e 100644
--- a/src/gallium/drivers/swr/rasterizer/core/backend_impl.h
+++ b/src/gallium/drivers/swr/rasterizer/core/backend_impl.h
@@ -62,10 +62,8 @@ static INLINE simdmask ComputeUserClipMask(uint8_t clipMask, 
float* pUserClipBuf
 simdscalar vB = _simd_broadcast_ss(pUserClipBuffer++);
 simdscalar vC = _simd_broadcast_ss(pUserClipBuffer++);
 
-simdscalar vK = _simd_sub_ps(_simd_sub_ps(_simd_set1_ps(1.0f), vI), 
vJ);
-
 // interpolate
-simdscalar vInterp = vplaneps(vA, vB, _simd_mul_ps(vK, vC), vI, vJ);
+simdscalar vInterp = vplaneps(vA, vB, vC, vI, vJ);
 
 // clip if interpolated clip distance is < 0 || NAN
 simdscalar vCull = _simd_cmp_ps(_simd_setzero_ps(), vInterp, 
_CMP_NLE_UQ);
diff --git a/src/gallium/drivers/swr/rasterizer/core/binner.cpp 
b/src/gallium/drivers/swr/rasterizer/core/binner.cpp
index 3b093ce..c9a37cb 100644
--- a/src/gallium/drivers/swr/rasterizer/core/binner.cpp
+++ b/src/gallium/drivers/swr/rasterizer/core/binner.cpp
@@ -256,12 +256,27 @@ void ProcessUserClipDist(const SWR_BACKEND_STATE& state, 
PA_STATE& pa, uint32_t
 simd4scalar primClipDist[3];
 pa.AssembleSingle(clipAttribSlot, primIndex, primClipDist);
 
+float vertClipDist[NumVerts];
 for (uint32_t e = 0; e < NumVerts; ++e)
 {
 OSALIGNSIMD(float) aVertClipDist[4];
 SIMD128::store_ps(aVertClipDist, primClipDist[e]);
-*(pUserClipBuffer++) = aVertClipDist[clipComp];
+vertClipDist[e] = aVertClipDist[clipComp];
 };
+
+// setup plane equations for barycentric interpolation in the backend
+float baryCoeff[NumVerts];
+float last = vertClipDist[NumVerts - 1] * pRecipW[NumVerts - 1];
+for (uint32_t e = 0; e < NumVerts - 1; ++e)
+{
+baryCoeff[e] = vertClipDist[e] * pRecipW[e] - last;
+}
+baryCoeff[NumVerts - 1] = last;
+
+for (uint32_t e = 0; e < NumVerts; ++e)
+{
+*(pUserClipBuffer++) = baryCoeff[e];
+}
 }
 }
 
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 4/5] swr/rast: Faster frustum prim culling

[George Kyriazis]

Fix clipper validMask setting. We don't need to run frustum rejected
primitives through the clipper.  Perform frustum culling with only
frustum clip codes. Guardband clip codes cannot be used because they
overlap frustum codes.
---
 src/gallium/drivers/swr/rasterizer/core/clip.h | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/swr/rasterizer/core/clip.h 
b/src/gallium/drivers/swr/rasterizer/core/clip.h
index 8d2590a..0f8399c 100644
--- a/src/gallium/drivers/swr/rasterizer/core/clip.h
+++ b/src/gallium/drivers/swr/rasterizer/core/clip.h
@@ -60,6 +60,7 @@ enum SWR_CLIPCODES
 };
 
 #define GUARDBAND_CLIP_MASK 
(FRUSTUM_NEAR|FRUSTUM_FAR|GUARDBAND_LEFT|GUARDBAND_TOP|GUARDBAND_RIGHT|GUARDBAND_BOTTOM|NEGW)
+#define FRUSTUM_CLIP_MASK 
(FRUSTUM_NEAR|FRUSTUM_FAR|FRUSTUM_LEFT|FRUSTUM_RIGHT|FRUSTUM_TOP|FRUSTUM_BOTTOM)
 
 template
 void ComputeClipCodes(const API_STATE &state, const Vec4 &vertex, 
Float &clipCodes, Integer const &viewportIndexes)
@@ -708,15 +709,18 @@ public:
 primMask &= ~ComputeUserClipCullMask(pa, prim);
 }
 
-// cull prims outside view frustum
 Float clipIntersection = ComputeClipCodeIntersection();
+// Mask out non-frustum codes
+clipIntersection = SIMD_T::and_ps(clipIntersection, 
SIMD_T::castsi_ps(SIMD_T::set1_epi32(FRUSTUM_CLIP_MASK)));
+
+// cull prims outside view frustum
 int validMask = primMask & 
SimdHelper::cmpeq_ps_mask(clipIntersection, SIMD_T::setzero_ps());
 
 // skip clipping for points
 uint32_t clipMask = 0;
 if (NumVertsPerPrim != 1)
 {
-clipMask = primMask & ComputeClipMask();
+clipMask = validMask & ComputeClipMask();
 }
 
 AR_EVENT(ClipInfoEvent(numInvoc, validMask, clipMask));
@@ -726,7 +730,7 @@ public:
 RDTSC_BEGIN(FEGuardbandClip, pa.pDC->drawId);
 // we have to clip tris, execute the clipper, which will also
 // call the binner
-ClipSimd(prim, SIMD_T::vmask_ps(primMask), 
SIMD_T::vmask_ps(clipMask), pa, primId, viewportIdx, rtIdx);
+ClipSimd(prim, SIMD_T::vmask_ps(validMask), 
SIMD_T::vmask_ps(clipMask), pa, primId, viewportIdx, rtIdx);
 RDTSC_END(FEGuardbandClip, 1);
 }
 else if (validMask)
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/5] swr/rast: Consolidate TRANSLATE_ADDRESS

[George Kyriazis]

Translate is now part of an overloaded LOAD call which required a change to
the code gen to skip the load functions in order to handle them manually
to make them virtual.
---
 .../swr/rasterizer/codegen/gen_llvm_ir_macros.py |  3 ++-
 .../drivers/swr/rasterizer/jitter/builder_mem.cpp| 20 
 .../drivers/swr/rasterizer/jitter/builder_mem.h  |  7 ++-
 .../drivers/swr/rasterizer/jitter/fetch_jit.cpp  |  4 
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/src/gallium/drivers/swr/rasterizer/codegen/gen_llvm_ir_macros.py 
b/src/gallium/drivers/swr/rasterizer/codegen/gen_llvm_ir_macros.py
index 3b19cb4..aab499b 100644
--- a/src/gallium/drivers/swr/rasterizer/codegen/gen_llvm_ir_macros.py
+++ b/src/gallium/drivers/swr/rasterizer/codegen/gen_llvm_ir_macros.py
@@ -152,7 +152,8 @@ def parse_ir_builder(input_file):
 # The following functions need to be ignored.
 if (func_name == 'CreateInsertNUWNSWBinOp' or
 func_name == 'CreateMaskedIntrinsic' or
-func_name == 'CreateAlignmentAssumptionHelper'):
+func_name == 'CreateAlignmentAssumptionHelper' or
+func_name == 'CreateLoad'):
 ignore = True
 
 # Convert CamelCase to CAMEL_CASE
diff --git a/src/gallium/drivers/swr/rasterizer/jitter/builder_mem.cpp 
b/src/gallium/drivers/swr/rasterizer/jitter/builder_mem.cpp
index f7d0402..05e450f 100644
--- a/src/gallium/drivers/swr/rasterizer/jitter/builder_mem.cpp
+++ b/src/gallium/drivers/swr/rasterizer/jitter/builder_mem.cpp
@@ -69,6 +69,26 @@ namespace SwrJit
 return IN_BOUNDS_GEP(ptr, indices);
 }
 
+LoadInst* Builder::LOAD(Value *Ptr, const char *Name)
+{
+return IRB()->CreateLoad(Ptr, Name);
+}
+
+LoadInst* Builder::LOAD(Value *Ptr, const Twine &Name)
+{
+return IRB()->CreateLoad(Ptr, Name);
+}
+
+LoadInst* Builder::LOAD(Type *Ty, Value *Ptr, const Twine &Name)
+{
+return IRB()->CreateLoad(Ty, Ptr, Name);
+}
+
+LoadInst* Builder::LOAD(Value *Ptr, bool isVolatile, const Twine &Name)
+{
+return IRB()->CreateLoad(Ptr, isVolatile, Name);
+}
+
 LoadInst *Builder::LOAD(Value *basePtr, const 
std::initializer_list &indices, const llvm::Twine& name)
 {
 std::vector valIndices;
diff --git a/src/gallium/drivers/swr/rasterizer/jitter/builder_mem.h 
b/src/gallium/drivers/swr/rasterizer/jitter/builder_mem.h
index 4f49634..b3a0e2b 100644
--- a/src/gallium/drivers/swr/rasterizer/jitter/builder_mem.h
+++ b/src/gallium/drivers/swr/rasterizer/jitter/builder_mem.h
@@ -34,7 +34,12 @@ Value *GEP(Value* ptr, const std::initializer_list 
&indexList);
 Value *IN_BOUNDS_GEP(Value* ptr, const std::initializer_list 
&indexList);
 Value *IN_BOUNDS_GEP(Value* ptr, const std::initializer_list 
&indexList);
 
-LoadInst *LOAD(Value *BasePtr, const std::initializer_list &offset, 
const llvm::Twine& name = "");
+virtual LoadInst* LOAD(Value *Ptr, const char *Name);
+virtual LoadInst* LOAD(Value *Ptr, const Twine &Name = "");
+virtual LoadInst* LOAD(Type *Ty, Value *Ptr, const Twine &Name = "");
+virtual LoadInst* LOAD(Value *Ptr, bool isVolatile, const Twine &Name = "");
+virtual LoadInst* LOAD(Value *BasePtr, const std::initializer_list 
&offset, const llvm::Twine& Name = "");
+
 LoadInst *LOADV(Value *BasePtr, const std::initializer_list &offset, 
const llvm::Twine& name = "");
 StoreInst *STORE(Value *Val, Value *BasePtr, const 
std::initializer_list &offset);
 StoreInst *STOREV(Value *Val, Value *BasePtr, const 
std::initializer_list &offset);
diff --git a/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp 
b/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
index 68bd4c1..f1dc002 100644
--- a/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
+++ b/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
@@ -1830,16 +1830,12 @@ Value* FetchJit::GetSimdValid16bitIndices(Value* 
pIndices, Value* pLastIndex)
 Value* pZeroIndex = ALLOCA(mInt16Ty);
 STORE(C((uint16_t)0), pZeroIndex);
 
-pLastIndex = TRANSLATE_ADDRESS(pLastIndex);
-
 // Load a SIMD of index pointers
 for(int64_t lane = 0; lane < mVWidth; lane++)
 {
 // Calculate the address of the requested index
 Value *pIndex = GEP(pIndices, C(lane));
 
-pIndex = TRANSLATE_ADDRESS(pIndex);
-
 // check if the address is less than the max index, 
 Value* mask = ICMP_ULT(pIndex, pLastIndex);
 
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] appveyor: Build with MSVC 2015.

[Jose Fonseca]

The MSVC version we (at VMware) primarily care about from now on is
2015.

We can drop support for building with 2013 in a future commit.  I'm not
aware of significant changes in C99/C11 support from MSVC 2013 to 2015,
but there's no point in continuing supporting old MSVC versions when
nobody cares.

Testing done: https://ci.appveyor.com/project/jrfonseca/mesa/build/46
---
 appveyor.yml | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/appveyor.yml b/appveyor.yml
index 96eb1a67b3..bd33e2e955 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -35,13 +35,13 @@ clone_depth: 100
 
 cache:
 - win_flex_bison-2.5.9.zip
-- llvm-3.3.1-msvc2013-mtd.7z
+- llvm-3.3.1-msvc2015-mtd.7z
 
-os: Visual Studio 2013
+os: Visual Studio 2015
 
 environment:
   WINFLEXBISON_ARCHIVE: win_flex_bison-2.5.9.zip
-  LLVM_ARCHIVE: llvm-3.3.1-msvc2013-mtd.7z
+  LLVM_ARCHIVE: llvm-3.3.1-msvc2015-mtd.7z
 
 install:
 # Check pip
@@ -69,10 +69,10 @@ install:
 - set LLVM=%CD%\llvm
 
 build_script:
-- scons -j%NUMBER_OF_PROCESSORS% MSVC_VERSION=12.0 llvm=1
+- scons -j%NUMBER_OF_PROCESSORS% MSVC_VERSION=14.0 llvm=1
 
 after_build:
-- scons -j%NUMBER_OF_PROCESSORS% MSVC_VERSION=12.0 llvm=1 check
+- scons -j%NUMBER_OF_PROCESSORS% MSVC_VERSION=14.0 llvm=1 check
 
 
 # It's possible to setup notification here, as described in
-- 
2.14.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/13] RadeonSI: Reduce user SGPR usage

[Marek Olšák]

I don't think that adding "uint32_t userdata_XX[16];" would simplify anything.

The bottom line is, patches 9-13 are prerequisites for VBO descriptors
in user SGPRs, so they block that optimization as long as they sit on
the mailing list.

Marek

On Tue, Feb 20, 2018 at 8:51 PM, Marek Olšák  wrote:
> The user SGPRs for blits are kinda a separate thing where the standard
> emit paths are disabled. 64-bit pointers are a short-term issue and
> will be removed in 2 years (or 1.5 years or when we want to kill off
> old LLVM support). VBO descriptors in user SGPRs will require 32-bit
> pointers. Next-gen will also require 32-bit pointers. The number of
> codepaths will be reduced to merged/non-merged and mono/non-mono
> again. For gfx9 and later, the only codepaths will be mono/non-mono.
>
> There will just be a transitory period when both 32-bit and 64-bit
> pointers will be supported, and both the old and new way of setting up
> VBO descriptors will be supported. However, next-gen will only support
> one way - the newer way.
>
> Overall, I don't see an increase in complexity other than the transitory 
> period.
>
> Marek
>
> On Tue, Feb 20, 2018 at 5:46 PM, Nicolai Hähnle  wrote:
>> With a small comment on patch 6, patches 1-8:
>>
>> Reviewed-by: Nicolai Hähnle 
>>
>> for now.
>>
>> However, I'm unhappy about how complex this is all getting. 32- vs. 64-bit,
>> merged vs. non-merged, monolithic vs. non-monolithic, and then special user
>> SGPR uses like for blits and soon VBO descriptors, it feels like it's
>> becoming too much.
>>
>> The problem is I don't have a good answer to it all :)
>>
>> Perhaps some of it could be helped by having an explicit userdata staging
>> area, i.e.
>>
>>   uint32_t userdata_XX[16]; // or 32
>>   uint32_t userdata_XX_dirty;
>>
>> Then si_upload_descriptors would write its pointers into userdata_XX in the
>> right location and set the appropriate dirty bit(s), and a separate
>> emit_userdata function would use the contiguous bit scan to actually emit
>> all the userdata together -- this would include VS state bits, tess state
>> info, and blit shader SGPRs.
>>
>> I do think this would be cleaner especially than the current
>> si_emit_shader_pointer_* code, and it would coalesce more SH reg writes as a
>> side bonus. What do you think?
>>
>> The other half of it is how the LLVM functions are created.
>>
>> Thanks,
>> Nicolai
>>
>>
>> On 17.02.2018 20:43, Marek Olšák wrote:
>>>
>>> Hi,
>>>
>>> This series has the following effect on user SGPRs:
>>>
>>> 64-bit pointers:
>>>  TCS:14 -> 12
>>>  Merged VS-TCS: 24 -> 20
>>>  Merged VS-GS:  18 -> 16
>>>  Merged TES-GS: 18 -> 14
>>>
>>> 32-bit pointers:
>>>  TCS:10 -> 8
>>>  Merged VS-TCS: 16 -> 12
>>>  Merged VS-GS:  11 -> 9
>>>  Merged TES-GS: 11 -> 6
>>>
>>> I tested both monolithic and non-monolithic shaders, and both 64-bit
>>> and 32-bit pointers. (4 combinations)
>>>
>>> This series is a prerequisite for VBO descriptors in user SGPRs.
>>>
>>> Note that merged LS-HS and ES-GS don't even use s[6:7] input SGPRs
>>> yet. Those only provide 40 bits of scalar data (not 64 bits like
>>> s[0:1]).
>>>
>>> Please review.
>>>
>>> Thanks,
>>> Marek
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
>>
>> --
>> Lerne, wie die Welt wirklich ist,
>> Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH v2] glx: Properly handle cases where screen creation fails

[Emil Velikov]

On 22 February 2018 at 15:18, Chuck Atkins  wrote:
>> On 22 February 2018 at 14:33, Chuck Atkins 
>> wrote:
>> > This fixes a segfault exposed by a29d63ecf7 which occurs when swr is
>> > used on an unsupported architecture.
>> >
>> > v2: re-work to place logic in xmesa_init_display
>> >
>> > Signed-off-by: Chuck Atkins 
>> > Cc: mesa-sta...@lists.freedesktop.org
>> > Cc: George Kyriazis 
>> > Cc: Bruce Cherniak 
>> FWIW
>> Reviewed-by: Emil Velikov 
>
>
> Thanks for the review.
>
>
>>
>> Gut feeling suggests that this and perhaps the choose_visual() hunks
>> are signs of other preexisting bugs.
>> If you decide to stick around with xlib-glx it is worth nuking the
>> XMesa abstraction/API.
>
>
> Our use case for building Mesa is to have as much of a self-contained
> software-only libGL and libOSMesa as possible, hence using the xlib-glx
> since using dri brings in a significant number of external dependencies.  We
> use it in HPC environments with no GPUs with OSMesa on the server-side and
> GLX on the client side, which ensures we can pre-package a GLX library for
> the GUI client that will support new GL specs while not needing to be too
> concerned about performance since the heavy lifting for rendering is done
> distributed on the server side.
>
> If there's a better way forward for having a minimal-dependency
> software-only implementation, I'd certainly be willing to try it.  At the
> moment though, gallium-xlib-glx is our path for that.
>
I was merely mentioning that the xlib-glx are in worse shape than the dri one.

That aside:
IIRC previously you mentioned that libGL/OSMesa must be static
libraries. If that is still the case, then DRI based GLX won't cut it.
Alternatively if you can point out any specifics that would be
amazing. Quick skim shows a few extras needed:

* GL side:
glapi + dri1/dri2 or dri3 specific: eg. xcb-dri2, xcb-dri3, xcb-present...
* [software] driver side:
glapi
* and glapi, itself
pthread, c

I've been wondering about 'splitting out' some of the DRI bits out,
although the topic is very subtle.
Depending on your specifics we might think of something though.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 03/17] intel: Disable 64-bit extensions on platforms without 64-bit types

[Iago Toral]

On Wed, 2018-02-21 at 09:33 -0800, Matt Turner wrote:
> On Wed, Feb 21, 2018 at 2:05 AM, Iago Toral 
> wrote:
> > On Tue, 2018-02-20 at 21:15 -0800, Matt Turner wrote:
> > > Gen11 does not support DF, Q, UQ types in hardware. As a result,
> > > we
> > > have
> > > to disable some GL extensions until they can be reimplemented.
> > > ---
> > >  src/intel/common/gen_device_info.c   | 3 +++
> > >  src/intel/common/gen_device_info.h   | 1 +
> > >  src/mesa/drivers/dri/i965/intel_extensions.c | 9 +
> > >  3 files changed, 9 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/intel/common/gen_device_info.c
> > > b/src/intel/common/gen_device_info.c
> > > index 8bf4b6b9bb0..465d4c783a1 100644
> > > --- a/src/intel/common/gen_device_info.c
> > > +++ b/src/intel/common/gen_device_info.c
> > > @@ -138,6 +138,7 @@ static const struct gen_device_info
> > > gen_device_info_snb_gt2 = {
> > > .must_use_separate_stencil = true,   \
> > > .has_llc = true, \
> > > .has_pln = true, \
> > > +   .has_64bit_types = true, \
> > > .has_surface_tile_offset = true, \
> > > .timestamp_frequency = 1250
> > > 
> > > @@ -322,6 +323,7 @@ static const struct gen_device_info
> > > gen_device_info_hsw_gt3 = {
> > > .has_llc = true, \
> > > .has_sample_with_hiz = false,\
> > > .has_pln = true, \
> > > +   .has_64bit_types = true, \
> > > .supports_simd16_3src = true,\
> > > .has_surface_tile_offset = true, \
> > > .max_vs_threads = 504,   \
> > > @@ -756,6 +758,7 @@ static const struct gen_device_info
> > > gen_device_info_cnl_5x8 = {
> > >  #define GEN11_FEATURES(_gt, _slices, _l3)   \
> > > GEN8_FEATURES,   \
> > > GEN11_HW_INFO,   \
> > > +   .has_64bit_types = false,@@ -322,6 +323,7 @@
> > > static
> > > const struct gen_device_info gen_device_info_hsw_gt3 = {
> > > \
> > > .gt = _gt, .num_slices = _slices, .l3_banks = _l3
> > 
> > This seems to only enable 64-bit types on snb_gt2 and hsw_gt3, what
> > about bdw+ platforms? or ivybridge where we also support fp64?
> 
> Sorry, I should have sent the diff with more context. git diff's
> context lines are useless, since we're modifying macros here:
> GEN7_FEATURES, GEN8_FEATURES, and GEN11_FEATURES (which includes
> GEN8_FEATURES).

Ah, that makes more sense, thanks!

This patch is:
Reviewed-by: Iago Toral Quiroga 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH v2] glx: Properly handle cases where screen creation fails

[Chuck Atkins]

>
> On 22 February 2018 at 14:33, Chuck Atkins 
> wrote:
> > This fixes a segfault exposed by a29d63ecf7 which occurs when swr is
> > used on an unsupported architecture.
> >
> > v2: re-work to place logic in xmesa_init_display
> >
> > Signed-off-by: Chuck Atkins 
> > Cc: mesa-sta...@lists.freedesktop.org
> > Cc: George Kyriazis 
> > Cc: Bruce Cherniak 
> FWIW
> Reviewed-by: Emil Velikov 
>

Thanks for the review.



> Gut feeling suggests that this and perhaps the choose_visual() hunks
> are signs of other preexisting bugs.
> If you decide to stick around with xlib-glx it is worth nuking the
> XMesa abstraction/API.
>

Our use case for building Mesa is to have as much of a self-contained
software-only libGL and libOSMesa as possible, hence using the xlib-glx
since using dri brings in a significant number of external dependencies.
We use it in HPC environments with no GPUs with OSMesa on the server-side
and GLX on the client side, which ensures we can pre-package a GLX library
for the GUI client that will support new GL specs while not needing to be
too concerned about performance since the heavy lifting for rendering is
done distributed on the server side.

If there's a better way forward for having a minimal-dependency
software-only implementation, I'd certainly be willing to try it.  At the
moment though, gallium-xlib-glx is our path for that.

- Chuck
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/5] tegra: Initial support

[Thierry Reding]

On Thu, Feb 22, 2018 at 02:31:48PM +0100, Erik Faye-Lund wrote:
> On Thu, Feb 22, 2018 at 2:23 PM, Thierry Reding
>  wrote:
> > On Wed, Feb 21, 2018 at 05:01:02PM +, Emil Velikov wrote:
> >> Hi Thierry,
> >>
> >> On 21 February 2018 at 15:30, Thierry Reding  
> >> wrote:
> >> > +static const char *
> >> > +tegra_screen_get_name(struct pipe_screen *pscreen)
> >> > +{
> >> > +   return "tegra";
> >> > +}
> >> > +
> >> > +static const char *
> >> > +tegra_screen_get_vendor(struct pipe_screen *pscreen)
> >> > +{
> >> > +   return "NVIDIA";
> >> > +}
> >> > +
> >> > +static const char *
> >> > +tegra_screen_get_device_vendor(struct pipe_screen *pscreen)
> >> > +{
> >> > +   return "NVIDIA";
> >> > +}
> >> > +
> >> As-is these might be a bit fiddly, but will do for now.
> >> For example - how will devs distinguish between the closed-source
> >> driver and Mesa.
> >
> > Good point. Let me check what exactly we use in the closed-source driver
> > and then come up with a proposal.
> >
> > I think perhaps a good choice for the vendor would be "grate". Even
> > though this driver isn't part of the grate project, I'm hoping that we
> > will eventually see Erik's and Dmitry's work merged into this.
> >
> > Adding Erik and Dmitry to get their opinion.
> >
> 
> It's not entirely clear to me what the most natural boundary between
> Tegra 2/3/4 (the GPUs the grate-project targets) and Tegra K1 would
> be. The later Tegras are so fundamentally different in how they
> work...
> 
> Should they share the implementation of tegra_screen_create? From a
> quick look, it doesn't seem like it (mostly K1 specifics going on
> there, it seems), but I could be wrong. And if it shouldn't perhaps it
> should simply be a separate gallium-driver ("grate" vs "tegra"
> maybe?)... We can probably share some code, but we can do that without
> sharing the driver, just like intel, amd and broadcom does with just a
> shared folder at src/nvidia or something for utilities...
> 
> I don't know, just trying to avoid having to add too many conditionals here...

I agree with you mostly. I don't think there's a lot that could be
shared here. On the other hand, I think we can easily add the
conditionals to only a few places. We could for example have a
grate_screen_create() function that is called by tegra_screen_create()
after detecting whether or not we have a Nouveau GPU in the system (as
fallback for a tegra_open_render_node() failure). Or we could do it
earlier after checking for Tegra chip ID in sysfs, or perhaps some DT
compatible string (via the drmDevice API).

That way there is one conditional in the initial stages, but we get to
deal with less fragmentation. I think it gives users a better experience
if they just install the Tegra driver and it works regardless of whether
it runs on a Tegra20 or a Tegra124.

We have a pretty consistent experience across all Tegra generations, and
I'd really like to keep that.

All of that said, I'm not exactly sure I understand the purpose of the
vendor name here. We already have the screen name that identifies this
as a Tegra screen and we have the device vendor which identifies who
built the device. So vendor seems to me like it would be whoever
provides the driver. In this case, couldn't that just be "Mesa"?

Thierry


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] swr/rast: Fix index buffer overfetch issue for non-indexed draws

[George Kyriazis]

Populate pLastIndex, even for the non-indexed case.  A zero pLastIndex
can cause the index offsets inside the fetcher to have non-sensical values
that can be either very large positive or very large negative numbers.

cc: mesa-sta...@lists.freedesktop.org
---
 src/gallium/drivers/swr/rasterizer/core/frontend.cpp| 15 +++
 src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/swr/rasterizer/core/frontend.cpp 
b/src/gallium/drivers/swr/rasterizer/core/frontend.cpp
index 9600f78..ce6cb68 100644
--- a/src/gallium/drivers/swr/rasterizer/core/frontend.cpp
+++ b/src/gallium/drivers/swr/rasterizer/core/frontend.cpp
@@ -1724,6 +1724,21 @@ void ProcessDraw(
 
 if (i < endVertex)
 {
+if (!IsIndexedT::value)
+{
+fetchInfo_lo.pLastIndex = fetchInfo_lo.pIndices;
+uint32_t offset;
+offset = std::min(endVertex-i, (uint32_t) 
KNOB_SIMD16_WIDTH);
+#if USE_SIMD16_SHADERS
+fetchInfo_lo.pLastIndex += offset;
+#else
+fetchInfo_lo.pLastIndex += std::min(offset, (uint32_t) 
KNOB_SIMD_WIDTH);
+uint32_t offset2 = std::min(offset, (uint32_t) 
KNOB_SIMD16_WIDTH)-KNOB_SIMD_WIDTH;
+assert(offset >= 0);
+fetchInfo_hi.pLastIndex = fetchInfo_hi.pIndices;
+fetchInfo_hi.pLastIndex += offset2;
+#endif
+}
 // 1. Execute FS/VS for a single SIMD.
 AR_BEGIN(FEFetchShader, pDC->drawId);
 #if USE_SIMD16_SHADERS
diff --git a/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp 
b/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
index 091db24..0bd2935 100644
--- a/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
+++ b/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
@@ -1872,7 +1872,7 @@ Value* FetchJit::GetSimdValid32bitIndices(Value* 
pIndices, Value* pLastIndex)
 // vIndexMask-1-1-1-1 0 0 0 0 : offsets < max pass
 // vLoadedIndices 0 1 2 3 0 0 0 0 : offsets >= max masked to 0
 Value* vMaxIndex = VBROADCAST(numIndicesLeft);
-Value* vIndexMask = VPCMPGTD(vMaxIndex,vIndexOffsets);
+Value* vIndexMask = VPCMPGTD(vMaxIndex, vIndexOffsets);
 
 // VMASKLOAD takes an *i8 src pointer
 pIndices = BITCAST(pIndices,PointerType::get(mInt8Ty,0));
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH v2] glx: Properly handle cases where screen creation fails

[Emil Velikov]

On 22 February 2018 at 14:33, Chuck Atkins  wrote:
> This fixes a segfault exposed by a29d63ecf7 which occurs when swr is
> used on an unsupported architecture.
>
> v2: re-work to place logic in xmesa_init_display
>
> Signed-off-by: Chuck Atkins 
> Cc: mesa-sta...@lists.freedesktop.org
> Cc: George Kyriazis 
> Cc: Bruce Cherniak 
FWIW
Reviewed-by: Emil Velikov 

> ---
>  src/gallium/state_trackers/glx/xlib/glx_api.c |  5 ++-
>  src/gallium/state_trackers/glx/xlib/xm_api.c  | 56 
> +--
>  src/gallium/state_trackers/glx/xlib/xm_api.h  |  2 +-
>  3 files changed, 33 insertions(+), 30 deletions(-)
>
> diff --git a/src/gallium/state_trackers/glx/xlib/glx_api.c 
> b/src/gallium/state_trackers/glx/xlib/glx_api.c
> index 1994e6823a..6ec3a31d5f 100644
> --- a/src/gallium/state_trackers/glx/xlib/glx_api.c
> +++ b/src/gallium/state_trackers/glx/xlib/glx_api.c
> @@ -743,7 +743,10 @@ choose_visual( Display *dpy, int screen, const int 
> *list, GLboolean fbConfig )
> int numAux = 0;
> GLint num_samples = 0;
>
> -   xmesa_init( dpy );
> +   if (xmesa_init( dpy ) != 0) {
> +  _mesa_warning(NULL, "Failed to initialize display");
> +  return NULL;
> +   }
>
> parselist = list;
>
> diff --git a/src/gallium/state_trackers/glx/xlib/xm_api.c 
> b/src/gallium/state_trackers/glx/xlib/xm_api.c
> index 934c0aba11..e8cbb618ce 100644
> --- a/src/gallium/state_trackers/glx/xlib/xm_api.c
> +++ b/src/gallium/state_trackers/glx/xlib/xm_api.c
> @@ -142,8 +142,11 @@ xmesa_close_display(Display *display)
>  {
> XMesaExtDisplayInfo *info, *prev;
>
> +   /* These assertions are not valid since screen creation can fail and 
> result
> +* in an empty list
> assert(MesaExtInfo.ndisplays > 0);
> assert(MesaExtInfo.head);
> +   */
>
Gut feeling suggests that this and perhaps the choose_visual() hunks
are signs of other preexisting bugs.
If you decide to stick around with xlib-glx it is worth nuking the
XMesa abstraction/API.

It hasn't been used in ages, it will help you simplify the whole thing
and squash a lot more lurking bugs.

HTH
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105171] performance regression (3x slower) running glamor with PutImage workload (radeonsi)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105171

Emil Velikov  changed:

   What|Removed |Added

   Keywords||bisected, regression
 CC||mar...@gmail.com

--- Comment #3 from Emil Velikov  ---
Thanks for the bisection Clemens.

For the future feel free to add the commit author/reviewer in the CC list. It
should help flag the issue amongst the dozens of others.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] glx: Properly handle cases where screen creation fails

[Chuck Atkins]

This fixes a segfault exposed by a29d63ecf7 which occurs when swr is
used on an unsupported architecture.

v2: re-work to place logic in xmesa_init_display

Signed-off-by: Chuck Atkins 
Cc: mesa-sta...@lists.freedesktop.org
Cc: George Kyriazis 
Cc: Bruce Cherniak 
---
 src/gallium/state_trackers/glx/xlib/glx_api.c |  5 ++-
 src/gallium/state_trackers/glx/xlib/xm_api.c  | 56 +--
 src/gallium/state_trackers/glx/xlib/xm_api.h  |  2 +-
 3 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/src/gallium/state_trackers/glx/xlib/glx_api.c 
b/src/gallium/state_trackers/glx/xlib/glx_api.c
index 1994e6823a..6ec3a31d5f 100644
--- a/src/gallium/state_trackers/glx/xlib/glx_api.c
+++ b/src/gallium/state_trackers/glx/xlib/glx_api.c
@@ -743,7 +743,10 @@ choose_visual( Display *dpy, int screen, const int *list, 
GLboolean fbConfig )
int numAux = 0;
GLint num_samples = 0;
 
-   xmesa_init( dpy );
+   if (xmesa_init( dpy ) != 0) {
+  _mesa_warning(NULL, "Failed to initialize display");
+  return NULL;
+   }
 
parselist = list;
 
diff --git a/src/gallium/state_trackers/glx/xlib/xm_api.c 
b/src/gallium/state_trackers/glx/xlib/xm_api.c
index 934c0aba11..e8cbb618ce 100644
--- a/src/gallium/state_trackers/glx/xlib/xm_api.c
+++ b/src/gallium/state_trackers/glx/xlib/xm_api.c
@@ -142,8 +142,11 @@ xmesa_close_display(Display *display)
 {
XMesaExtDisplayInfo *info, *prev;
 
+   /* These assertions are not valid since screen creation can fail and result
+* in an empty list
assert(MesaExtInfo.ndisplays > 0);
assert(MesaExtInfo.head);
+   */
 
_XLockMutex(_Xglobal_lock);
/* first find display */
@@ -223,7 +226,30 @@ xmesa_init_display( Display *display )
   return NULL;
}
info->display = display;
+
xmdpy = &info->mesaDisplay; /* to be filled out below */
+   xmdpy->display = display;
+   xmdpy->pipe = NULL;
+
+   xmdpy->smapi = CALLOC_STRUCT(st_manager);
+   if (!xmdpy->smapi) {
+  Xfree(info);
+  mtx_unlock(&init_mutex);
+  return NULL;
+   }
+
+   xmdpy->screen = driver.create_pipe_screen(display);
+   if (!xmdpy->screen) {
+  free(xmdpy->smapi);
+  Xfree(info);
+  mtx_unlock(&init_mutex);
+  return NULL;
+   }
+
+   /* At this point, both smapi and screen are known to be valid */
+   xmdpy->smapi->screen = xmdpy->screen;
+   xmdpy->smapi->get_param = xmesa_get_param;
+   (void) mtx_init(&xmdpy->mutex, mtx_plain);
 
/* chain to the list of displays */
_XLockMutex(_Xglobal_lock);
@@ -232,32 +258,6 @@ xmesa_init_display( Display *display )
MesaExtInfo.ndisplays++;
_XUnlockMutex(_Xglobal_lock);
 
-   /* now create the new XMesaDisplay info */
-   assert(display);
-
-   xmdpy->display = display;
-   xmdpy->screen = driver.create_pipe_screen(display);
-   xmdpy->smapi = CALLOC_STRUCT(st_manager);
-   xmdpy->pipe = NULL;
-   if (xmdpy->smapi) {
-  xmdpy->smapi->screen = xmdpy->screen;
-  xmdpy->smapi->get_param = xmesa_get_param;
-   }
-
-   if (xmdpy->screen && xmdpy->smapi) {
-  (void) mtx_init(&xmdpy->mutex, mtx_plain);
-   }
-   else {
-  if (xmdpy->screen) {
- xmdpy->screen->destroy(xmdpy->screen);
- xmdpy->screen = NULL;
-  }
-  free(xmdpy->smapi);
-  xmdpy->smapi = NULL;
-
-  xmdpy->display = NULL;
-   }
-
mtx_unlock(&init_mutex);
 
return xmdpy;
@@ -935,10 +935,10 @@ xmesa_get_name(void)
 /**
  * Do per-display initializations.
  */
-void
+int
 xmesa_init( Display *display )
 {
-   xmesa_init_display(display);
+   return xmesa_init_display(display) ? 0 : 1;
 }
 
 
diff --git a/src/gallium/state_trackers/glx/xlib/xm_api.h 
b/src/gallium/state_trackers/glx/xlib/xm_api.h
index 06bf839c96..e420679313 100644
--- a/src/gallium/state_trackers/glx/xlib/xm_api.h
+++ b/src/gallium/state_trackers/glx/xlib/xm_api.h
@@ -359,7 +359,7 @@ struct xmesa_buffer {
 extern const char *
 xmesa_get_name(void);
 
-extern void
+extern int
 xmesa_init(Display *dpy);
 
 extern XMesaBuffer
-- 
2.14.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/5] tegra: Initial support

[Emil Velikov]

On 22 February 2018 at 13:23, Thierry Reding  wrote:
> On Wed, Feb 21, 2018 at 05:01:02PM +, Emil Velikov wrote:
>> Hi Thierry,
>>
>> On 21 February 2018 at 15:30, Thierry Reding  
>> wrote:
>>
>> > @@ -2595,6 +2596,11 @@ if test -n "$with_gallium_drivers"; then
>> > ximx)
>> >  HAVE_GALLIUM_IMX=yes
>> >  ;;
>> > +xtegra)
>> > +HAVE_GALLIUM_TEGRA=yes
>> > +PKG_CHECK_MODULES([TEGRA], [libdrm_tegra >= 
>> > $LIBDRM_TEGRA_REQUIRED])
>> > +require_libdrm "tegra"
>> > +;;
>>
>> Could use the following hunk just after the "... needed dependencies
>> for renderonly drivers." comment.
>>
>> if test "x$HAVE_GALLIUM_NOUVEAU" != xyes -a "x$HAVE_GALLIUM_TEGRA" =
>> xyes  ; then
>> AC_MSG_ERROR([Building with tegra requires nouveau])
>> fi
>
> Done. It also turns out that we don't need libdrm_tegra (yet), so I can
> drop that, which makes it easier to build-test.
>
Great - one less thing to think about ;-)

>>
>> In a future patch we can:
>> - add tegra to the Makefile.am DISTCHECK list
>
> I can do that. With the libdrm_tegra dependency gone that should be no
> problem. But what if we want to add it back at some point? Are people
> supposed to have all the dependencies installed for a make distcheck?
>
>> - wire up the driver with .travis
>
> I'm not very familiar with Travis CI on Mesa. Is there a public instance
> that I can check? Also, is there a way to test such a change before
> pushing it to make sure we don't inadvertently break it?
>
> Well, I guess that's kind of the point of Travis, but I means things
> like making sure the .travis syntax and build dependencies are correct,
> and so on.
>
Don't worry, I'll address those. With a comment what to tweak if
libdrm_tegra becomes a dependency.


>
> Good point. Let me check what exactly we use in the closed-source driver
> and then come up with a proposal.
>
> I think perhaps a good choice for the vendor would be "grate". Even
> though this driver isn't part of the grate project, I'm hoping that we
> will eventually see Erik's and Dmitry's work merged into this.
>
> Adding Erik and Dmitry to get their opinion.
>
Ack. Since this can be tweaked later, I'd suggest not blocking the
series on the name specifics.


>> > +
>> > +  if ((device->available_nodes & (1 << DRM_NODE_RENDER)) &&
>> > +  (device->bustype == DRM_BUS_PLATFORM)) {
>> Worth checking for any instances of "tegra" in the platform/deviceinfo 
>> strings?
>
> This is actually checking for the Tegra GPU driven by Nouveau. The
> "tegra" device would be on DRM_BUS_HOST1X. And we kind of know that
> there will always only be one GPU on the platform bus.
>
> I could add a check for the driver, just not sure it's worth it.
>
My train of thought is as follows, although it might be tad pedantic.
Like in the kernel HW enablement patches are allowed in -stable. Yet
trying to retroactively fix a glitch is more fiddly.

Regardless, it's just an idea.


>> > +struct pipe_screen *
>> > +tegra_screen_create(int fd)
>> > +{
>> > +   struct tegra_screen *screen;
>> > +
>> > +   screen = calloc(1, sizeof(*screen));
>> > +   if (!screen)
>> > +  return NULL;
>> > +
>> > +   screen->fd = fd;
>> IIRC there were some fallouts for xinerama setups that required a dup().
>> Don't recall the details and how applicable those are here.
>
> Others seem to be doing this as well, though usually via the winsys
> rather than the driver. Looks like etnaviv does a regular dup() but
> vc4 does it via fcntl() to add O_CLOEXEC. The latter seems safer to
> me. I'll go with that.
>
As you can see the winsys split isn't as distinct on some drivers, so
trying to cross check is fiddly.
Regardless which option you go for, a small TODO/NOTE might be a good idea.


HTH
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105208] [regression] (e5ff036c67 st/dri: Add support for BGR[A/X]1010102 formats) broke the mouse in mutter/shell

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105208

Emil Velikov  changed:

   What|Removed |Added

 CC||airl...@freedesktop.org,
   ||jad...@gmail.com

--- Comment #2 from Emil Velikov  ---
Marek put forward a request about fixing gnome-shell/mutter although it never
got a reply from Dave or Jonas.

https://lists.freedesktop.org/archives/mesa-dev/2017-September/168921.html

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] intel: aubinator_error_decode: fix segfault on missing register

[Lionel Landwerlin]

Some register might be missing in our genxmls. Don't try to decode
them.

Signed-off-by: Lionel Landwerlin 
---
 src/intel/tools/aubinator_error_decode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/intel/tools/aubinator_error_decode.c 
b/src/intel/tools/aubinator_error_decode.c
index 2331114b446..08ece771fde 100644
--- a/src/intel/tools/aubinator_error_decode.c
+++ b/src/intel/tools/aubinator_error_decode.c
@@ -65,9 +65,9 @@ print_head(unsigned int reg)
 static void
 print_register(struct gen_spec *spec, const char *name, uint32_t reg)
 {
-   struct gen_group *reg_spec = gen_spec_find_register_by_name(spec, name);
+   struct gen_group *reg_spec;
 
-   if (reg_spec) {
+   if (name && (reg_spec = gen_spec_find_register_by_name(spec, name))) {
   gen_print_group(stdout, reg_spec, 0, ®, 0,
   option_color == COLOR_ALWAYS);
}
-- 
2.16.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/5] tegra: Initial support

[Erik Faye-Lund]

On Thu, Feb 22, 2018 at 2:23 PM, Thierry Reding
 wrote:
> On Wed, Feb 21, 2018 at 05:01:02PM +, Emil Velikov wrote:
>> Hi Thierry,
>>
>> On 21 February 2018 at 15:30, Thierry Reding  
>> wrote:
>> > +static const char *
>> > +tegra_screen_get_name(struct pipe_screen *pscreen)
>> > +{
>> > +   return "tegra";
>> > +}
>> > +
>> > +static const char *
>> > +tegra_screen_get_vendor(struct pipe_screen *pscreen)
>> > +{
>> > +   return "NVIDIA";
>> > +}
>> > +
>> > +static const char *
>> > +tegra_screen_get_device_vendor(struct pipe_screen *pscreen)
>> > +{
>> > +   return "NVIDIA";
>> > +}
>> > +
>> As-is these might be a bit fiddly, but will do for now.
>> For example - how will devs distinguish between the closed-source
>> driver and Mesa.
>
> Good point. Let me check what exactly we use in the closed-source driver
> and then come up with a proposal.
>
> I think perhaps a good choice for the vendor would be "grate". Even
> though this driver isn't part of the grate project, I'm hoping that we
> will eventually see Erik's and Dmitry's work merged into this.
>
> Adding Erik and Dmitry to get their opinion.
>

It's not entirely clear to me what the most natural boundary between
Tegra 2/3/4 (the GPUs the grate-project targets) and Tegra K1 would
be. The later Tegras are so fundamentally different in how they
work...

Should they share the implementation of tegra_screen_create? From a
quick look, it doesn't seem like it (mostly K1 specifics going on
there, it seems), but I could be wrong. And if it shouldn't perhaps it
should simply be a separate gallium-driver ("grate" vs "tegra"
maybe?)... We can probably share some code, but we can do that without
sharing the driver, just like intel, amd and broadcom does with just a
shared folder at src/nvidia or something for utilities...

I don't know, just trying to avoid having to add too many conditionals here...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/5] tegra: Initial support

[Thierry Reding]

On Wed, Feb 21, 2018 at 05:01:02PM +, Emil Velikov wrote:
> Hi Thierry,
> 
> On 21 February 2018 at 15:30, Thierry Reding  wrote:
> 
> > @@ -2595,6 +2596,11 @@ if test -n "$with_gallium_drivers"; then
> > ximx)
> >  HAVE_GALLIUM_IMX=yes
> >  ;;
> > +xtegra)
> > +HAVE_GALLIUM_TEGRA=yes
> > +PKG_CHECK_MODULES([TEGRA], [libdrm_tegra >= 
> > $LIBDRM_TEGRA_REQUIRED])
> > +require_libdrm "tegra"
> > +;;
> 
> Could use the following hunk just after the "... needed dependencies
> for renderonly drivers." comment.
> 
> if test "x$HAVE_GALLIUM_NOUVEAU" != xyes -a "x$HAVE_GALLIUM_TEGRA" =
> xyes  ; then
> AC_MSG_ERROR([Building with tegra requires nouveau])
> fi

Done. It also turns out that we don't need libdrm_tegra (yet), so I can
drop that, which makes it easier to build-test.

> 
> In a future patch we can:
> - add tegra to the Makefile.am DISTCHECK list

I can do that. With the libdrm_tegra dependency gone that should be no
problem. But what if we want to add it back at some point? Are people
supposed to have all the dependencies installed for a make distcheck?

> - wire up the driver with .travis

I'm not very familiar with Travis CI on Mesa. Is there a public instance
that I can check? Also, is there a way to test such a change before
pushing it to make sure we don't inadvertently break it?

Well, I guess that's kind of the point of Travis, but I means things
like making sure the .travis syntax and build dependencies are correct,
and so on.

> > index ..910cbe02d873
> > --- /dev/null
> > +++ b/include/drm-uapi/tegra_drm.h
> 
> Haven't checked this one, but I trust you picked the latest and greatest.

Yeah, this is from v4.16-rc1, though I've updated the include guards to
drop the UAPI from Linux' headers. That seemed to be what others were
doing, so hopefully that was correct.

> > --- /dev/null
> > +++ b/src/gallium/drivers/tegra/Makefile.am
> > @@ -0,0 +1,18 @@
> > +AUTOMAKE_OPTIONS = subdir-objects
> > +
> Not needed - already set at global scope.

Done.

> 
> > +include Makefile.sources
> > +include $(top_srcdir)/src/gallium/Automake.inc
> > +
> > +AM_CFLAGS = \
> > +   -I$(top_srcdir)/include/drm-uapi \
> > +   $(GALLIUM_DRIVER_CFLAGS) \
> > +   $(LIBUDEV_CFLAGS) \
> libudev is gone.

Done. I also removed the TEGRA_CFLAGS which is not (yet) needed.

> 
> > +
> > +noinst_LTLIBRARIES = libtegra.la
> > +
> > +libtegra_la_SOURCES = \
> > +   $(C_SOURCES)
> > +
> > +libtegra_la_LIBADD = \
> > +   $(LIBUDEV_LIBS)
> Ditto.

Done.

> 
> 
> > --- /dev/null
> > +++ b/src/gallium/drivers/tegra/Makefile.sources
> > @@ -0,0 +1,3 @@
> > +C_SOURCES := \
> > +   tegra_context.c \
> > +   tegra_screen.c
> 
> Please include the following to as well.
> tegra_context.h
> tegra_resource.h

Done.

> > index ..feaa5138c95d
> > --- /dev/null
> > +++ b/src/gallium/drivers/tegra/tegra_context.c
> 
> > +#include "tegra/tegra_context.h"
> > +#include "tegra/tegra_resource.h"
> > +#include "tegra/tegra_screen.h"
> > +
> The "tegra/" prefix isn't needed - it doesn't hurt though.

Removed.

> > --- /dev/null
> > +++ b/src/gallium/drivers/tegra/tegra_screen.c
> 
> > +#include "tegra/tegra_context.h"
> > +#include "tegra/tegra_resource.h"
> > +#include "tegra/tegra_screen.h"
> > +
> Ditto.

Removed.

> > +static const char *
> > +tegra_screen_get_name(struct pipe_screen *pscreen)
> > +{
> > +   return "tegra";
> > +}
> > +
> > +static const char *
> > +tegra_screen_get_vendor(struct pipe_screen *pscreen)
> > +{
> > +   return "NVIDIA";
> > +}
> > +
> > +static const char *
> > +tegra_screen_get_device_vendor(struct pipe_screen *pscreen)
> > +{
> > +   return "NVIDIA";
> > +}
> > +
> As-is these might be a bit fiddly, but will do for now.
> For example - how will devs distinguish between the closed-source
> driver and Mesa.

Good point. Let me check what exactly we use in the closed-source driver
and then come up with a proposal.

I think perhaps a good choice for the vendor would be "grate". Even
though this driver isn't part of the grate project, I'm hoping that we
will eventually see Erik's and Dmitry's work merged into this.

Adding Erik and Dmitry to get their opinion.

> > +static int tegra_open_render_node(void)
> > +{
> > +   drmDevicePtr *devices, device;
> > +   int err, fd = -ENODEV;
> > +   unsigned int num, i;
> > +
> > +   err = drmGetDevices2(0, NULL, 0);
> > +   if (err < 0)
> > +  return err;
> > +
> > +   num = err;
> > +
> > +   devices = calloc(num, sizeof(*devices));
> > +   if (!devices)
> > +  return -ENOMEM;
> > +
> > +   err = drmGetDevices2(0, devices, num);
> > +   if (err < 0) {
> > +  free(devices);
> > +  return err;
> > +   }
> > +
> > +   for (i = 0; i < num; i++) {
> > +  device = devices[i];
> > +
> > +  if ((device->available_nodes & (1 << DRM_NODE_RENDER)) &&
> > +  (device->bustype == DRM_BUS_PLATFORM)) {
> Worth checking fo

[Mesa-dev] [Bug 105161] Validation of KHR_blend_equation_advanced stricter than NVidia

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105161

--- Comment #9 from Allan Sandfeld Jensen  ---
The problem is the check in linker.cpp (link_fs_inout_layout_qualifiers):

   if (linked_shader->Stage != MESA_SHADER_FRAGMENT ||
   (prog->data->Version < 150 &&
!prog->ARB_fragment_coord_conventions_enable))
  return;

-- 
You are receiving this mail because:
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nv50,nvc0: fix clear buffer acceleration

[Karol Herbst]

Tested-by: Karol Herbst 

On Wed, Feb 21, 2018 at 5:20 AM, Ilia Mirkin  wrote:
> Two things were off:
>  - valid range was not updated, which could affect waiting for future
>maps
>  - fencing was done manually instead of using the *_resource_validate
>helper, which resulted in a missed dirty buffer flag being set
>
> Fixes: KHR-GL45.direct_state_access.buffers_clear
> Signed-off-by: Ilia Mirkin 
> ---
>
> Untested on pre-kepler paths. Pretty similar overall.
>
>  src/gallium/drivers/nouveau/nv50/nv50_surface.c | 20 
>  src/gallium/drivers/nouveau/nvc0/nvc0_surface.c | 25 
> +
>  2 files changed, 17 insertions(+), 28 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c 
> b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> index 908c534b92e..037e14a4d60 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> @@ -672,10 +672,7 @@ nv50_clear_buffer_push(struct pipe_context *pipe,
>count -= nr;
> }
>
> -   if (buf->mm) {
> -  nouveau_fence_ref(nv50->screen->base.fence.current, &buf->fence);
> -  nouveau_fence_ref(nv50->screen->base.fence.current, &buf->fence_wr);
> -   }
> +   nv50_resource_validate(buf, NOUVEAU_BO_WR);
>
> nouveau_bufctx_reset(nv50->bufctx, 0);
>  }
> @@ -727,6 +724,8 @@ nv50_clear_buffer(struct pipe_context *pipe,
>return;
> }
>
> +   util_range_add(&buf->valid_buffer_range, offset, offset + size);
> +
> assert(size % data_size == 0);
>
> if (offset & 0xff) {
> @@ -747,10 +746,10 @@ nv50_clear_buffer(struct pipe_context *pipe,
> assert(width > 0);
>
> BEGIN_NV04(push, NV50_3D(CLEAR_COLOR(0)), 4);
> -   PUSH_DATAf(push, color.f[0]);
> -   PUSH_DATAf(push, color.f[1]);
> -   PUSH_DATAf(push, color.f[2]);
> -   PUSH_DATAf(push, color.f[3]);
> +   PUSH_DATA (push, color.ui[0]);
> +   PUSH_DATA (push, color.ui[1]);
> +   PUSH_DATA (push, color.ui[2]);
> +   PUSH_DATA (push, color.ui[3]);
>
> if (nouveau_pushbuf_space(push, 64, 1, 0))
>return;
> @@ -796,10 +795,7 @@ nv50_clear_buffer(struct pipe_context *pipe,
> BEGIN_NV04(push, NV50_3D(COND_MODE), 1);
> PUSH_DATA (push, nv50->cond_condmode);
>
> -   if (buf->mm) {
> -  nouveau_fence_ref(nv50->screen->base.fence.current, &buf->fence);
> -  nouveau_fence_ref(nv50->screen->base.fence.current, &buf->fence_wr);
> -   }
> +   nv50_resource_validate(buf, NOUVEAU_BO_WR);
>
> if (width * height != elements) {
>offset += width * height * data_size;
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
> index 9445c05f3ab..0f86c11b7f4 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
> @@ -403,10 +403,7 @@ nvc0_clear_buffer_push_nvc0(struct pipe_context *pipe,
>size -= nr * 4;
> }
>
> -   if (buf->mm) {
> -  nouveau_fence_ref(nvc0->screen->base.fence.current, &buf->fence);
> -  nouveau_fence_ref(nvc0->screen->base.fence.current, &buf->fence_wr);
> -   }
> +   nvc0_resource_validate(buf, NOUVEAU_BO_WR);
>
> nouveau_bufctx_reset(nvc0->bufctx, 0);
>  }
> @@ -453,10 +450,7 @@ nvc0_clear_buffer_push_nve4(struct pipe_context *pipe,
>size -= nr * 4;
> }
>
> -   if (buf->mm) {
> -  nouveau_fence_ref(nvc0->screen->base.fence.current, &buf->fence);
> -  nouveau_fence_ref(nvc0->screen->base.fence.current, &buf->fence_wr);
> -   }
> +   nvc0_resource_validate(buf, NOUVEAU_BO_WR);
>
> nouveau_bufctx_reset(nvc0->bufctx, 0);
>  }
> @@ -540,6 +534,8 @@ nvc0_clear_buffer(struct pipe_context *pipe,
>return;
> }
>
> +   util_range_add(&buf->valid_buffer_range, offset, offset + size);
> +
> assert(size % data_size == 0);
>
> if (data_size == 12) {
> @@ -570,10 +566,10 @@ nvc0_clear_buffer(struct pipe_context *pipe,
> PUSH_REFN (push, buf->bo, buf->domain | NOUVEAU_BO_WR);
>
> BEGIN_NVC0(push, NVC0_3D(CLEAR_COLOR(0)), 4);
> -   PUSH_DATAf(push, color.f[0]);
> -   PUSH_DATAf(push, color.f[1]);
> -   PUSH_DATAf(push, color.f[2]);
> -   PUSH_DATAf(push, color.f[3]);
> +   PUSH_DATA (push, color.ui[0]);
> +   PUSH_DATA (push, color.ui[1]);
> +   PUSH_DATA (push, color.ui[2]);
> +   PUSH_DATA (push, color.ui[3]);
> BEGIN_NVC0(push, NVC0_3D(SCREEN_SCISSOR_HORIZ), 2);
> PUSH_DATA (push, width << 16);
> PUSH_DATA (push, height << 16);
> @@ -600,10 +596,7 @@ nvc0_clear_buffer(struct pipe_context *pipe,
>
> IMMED_NVC0(push, NVC0_3D(COND_MODE), nvc0->cond_condmode);
>
> -   if (buf->mm) {
> -  nouveau_fence_ref(nvc0->screen->base.fence.current, &buf->fence);
> -  nouveau_fence_ref(nvc0->screen->base.fence.current, &buf->fence_wr);
> -   }
> +   nvc0_resource_validate(buf, NOUVEAU_BO_WR);
>
> if (width * height != elements) {
>offset += width * height * data_size;
> --
> 2.16.1
>
> ___

Re: [Mesa-dev] [PATCH] nv50, nvc0: fix integer MS resolves using 2d engine

[Karol Herbst]

Tested-by: Karol Herbst 

On Thu, Feb 22, 2018 at 5:34 AM, Ilia Mirkin  wrote:
> We don't want filtering for integer textures, same as depth/stencil.
>
> Fixes: KHR-GL45.direct_state_access.renderbuffers_storage_multisample
> Signed-off-by: Ilia Mirkin 
> ---
>  src/gallium/drivers/nouveau/nv50/nv50_blit.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_blit.h 
> b/src/gallium/drivers/nouveau/nv50/nv50_blit.h
> index 0ccec568d3a..10fe5274b10 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_blit.h
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_blit.h
> @@ -85,7 +85,8 @@ static inline unsigned
>  nv50_blit_get_filter(const struct pipe_blit_info *info)
>  {
> if (info->dst.resource->nr_samples < info->src.resource->nr_samples)
> -  return util_format_is_depth_or_stencil(info->src.format) ? 0 : 1;
> +  return (util_format_is_depth_or_stencil(info->src.format) ||
> +  util_format_is_pure_integer(info->src.format)) ? 0 : 1;
>
> if (info->filter != PIPE_TEX_FILTER_LINEAR)
>return 0;
> --
> 2.16.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nvc0: fix writing query results into buffer

[Karol Herbst]

Tested-by: Karol Herbst 

On Wed, Feb 21, 2018 at 6:17 AM, Ilia Mirkin  wrote:
> We need to mark the range as valid, and validate the resource using a
> helper to ensure that the buffer status is marked properly.
>
> Fixes some CTS pipeline stats query tests.
>
> Signed-off-by: Ilia Mirkin 
> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_query_hw.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw.c
> index 7568eeb94db..ef5f939319a 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw.c
> @@ -473,10 +473,10 @@ nvc0_hw_get_query_result_resource(struct nvc0_context 
> *nvc0,
> PUSH_DATAh(push, buf->address + offset);
> PUSH_DATA (push, buf->address + offset);
>
> -   if (buf->mm) {
> -  nouveau_fence_ref(nvc0->screen->base.fence.current, &buf->fence);
> -  nouveau_fence_ref(nvc0->screen->base.fence.current, &buf->fence_wr);
> -   }
> +   util_range_add(&buf->valid_buffer_range, offset,
> +  offset + (result_type >= PIPE_QUERY_TYPE_I64 ? 8 : 4));
> +
> +   nvc0_resource_validate(buf, NOUVEAU_BO_WR);
>  }
>
>  static const struct nvc0_query_funcs hw_query_funcs = {
> --
> 2.16.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105161] Validation of KHR_blend_equation_advanced stricter than NVidia

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105161

--- Comment #8 from Allan Sandfeld Jensen  ---
That would fix the parsing in compatibility shaders. It doesn't make it work.

I found after digging into mesa code that I needed:

#extension GL_ARB_fragment_coord_conventions : enable

Otherwise the layout qualifiers are only passed, but not passed on. I am not
sure if that is a bug, but there was nothing in the spec that suggested you
need to manually activate that extension as well to get it the layout qualifier
to work.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105208] [regression] (e5ff036c67 st/dri: Add support for BGR[A/X]1010102 formats) broke the mouse in mutter/shell

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105208

Daniel Stone  changed:

   What|Removed |Added

 CC||mar...@gmail.com,
   ||mario.klei...@tuebingen.mpg
   ||.de

--- Comment #1 from Daniel Stone  ---
The ultimate root cause is that Mutter expects all its configs are 8bpc, but
asks EGL for 'anything at least 1bpc' through eglChooseConfigs, which is
explicitly specified to return deeper channels if available; i.e. 10bpc must be
preferred to 8bpc if available.

The way it handles mouse input is that it renders the scene with every surface
as its own solid colour, reads back to find out what colour is at the current
mouse position, and maps that back to a surface. It's unaware of 10bpc when
doing so, and so the colour never matches back to any surface.

Since quite a few places are broken in the same way, I'm suspecting we should
probably change the allow_rgb10_configs default to false, at least until we can
get some of the worst offenders fixed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105208] [regression] (e5ff036c67 st/dri: Add support for BGR[A/X]1010102 formats) broke the mouse in mutter/shell

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105208

Bug ID: 105208
   Summary: [regression] (e5ff036c67 st/dri: Add support for
BGR[A/X]1010102 formats) broke the mouse in
mutter/shell
   Product: Mesa
   Version: unspecified
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Other
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: yan...@declera.com
QA Contact: mesa-dev@lists.freedesktop.org

Bisected a problem with this commit

e5ff036c67 st/dri: Add support for BGR[A/X]1010102 formats

That broke the mouse input in mutter/shell in Fedora rawhide.

I am clueless as to why.

See:
https://bugzilla.redhat.com/show_bug.cgi?id=1547691

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [ANNOUNCE] Mesa 17.3.4 release candidate

[Emil Velikov]

On 21 February 2018 at 19:14, Kenneth Graunke  wrote:
> On Thursday, February 8, 2018 8:47:00 PM PST Emil Velikov wrote:
>> Rejected (9)
>> 
>> Jason Ekstrand (2):
>>   e52a9f18d69c94b7cb7f81361cdb9e2582c3d742 i965: Replace
>> draw_aux_buffer_disabled with draw_aux_usage
>>   20f70ae3858bc213e052a8434f0e637eb36203c4 i965/draw: Set
>> NEW_AUX_STATE when draw aux changes
>> Reason: Introduce multiple regressions in the piglit compute shader tests.
>
> Hi Emil,
>
Hi Ken,

> These are absolutely critical fixes.  These patches fix GPU hangs and
> crashes in Glamor which cause people's X session to die when doing
> exciting things like using their text editor, IDE, or desktop panel.
> It's responsible for a huge swath of our GPU hang bugs on i965.
>
> Did Jason or I miss an email from you about these being rejected,
> other than at the bottom of a large changelog in an RC announcement?
> Which Piglit tests are regressing?  My guess is that we just need to
> nominate another patch, as they aren't broken in master.
>
You're right, I should have included more specifics.

The commits cause approx. 1700 regressions in the following:
spec/amd_shader_trinary_minmax/execution/built-in-functions/cs-*
spec/arb_compute_shader/execution/*
spec/arb_gpu_shader_int64/execution/built-in-functions/cs-*
spec/glsl-4.30/execution/built-in-functions/cs-*


I've a dozen of attempts trying to find the missing patch(es).
I _really_ want the patches to land, see [1].

As a rule the author of a rejected patch or one with merge conflicts
is explicitly CCed in the RC email.
Additionally, there is also a reply[2] to the patch itself with
request for a)information and/or b) backport.

Yes, we can remind developers more frequently. Yet at some point it
only gets annoying and ultimately - ignored.
Suggestions are more than welcome.


> At this point, we've done 5 point releases in the 17.3.x series, which
> have had DRI3 crashes when pageflipping (in all drivers), and X server
> hangs and crashes galore in i965/Gen9+.  Worse, we fixed those hangs a
> month ago and haven't managed to ship them yet.  We also managed to
> ship a radv that broke completely.
>
> At this point, 17.3.x is looking like the worst Mesa release in recent
> memory, and I'm about on the verge of advising people to just go back
> to 17.2 until 18.0 comes out.  It's pretty frustrating, and I feel bad
> for our users, who depend on our software for their computer to work.
>
According to the results from the Jenkins setup, there are no
regressions in 17.3.x wrt the 17.2.x series.
Perhaps we lack test coverage?

Additionally I would not call for 17.2 since I did notice some
glitches with it and Tomb Rider and Dota2.
Latter triggered by a Dota2 update.

> We have to do better, somehow - myself included.  Ideally, we'd find a
> way to avoid major bugs like this in the first place.  Barring that,
> do we need to have developers take a more active role in backporting
> fixes again?  It seems like our nomination process works for simple
> things, but for more complex series, it doesn't work as well.  Maybe
> we need to proactively put together (tested) pull requests for stable?
>
Hear, hear (aka yes please) for more developer backports.

Should be a good idea to also cross review for the conflicts that
myself or the Igalia team resolve.

Obviously that should not substitute testing _and_ reporting from the
different teams.
Currently the _only_ information that we have is from the Jenkins CI.

Thanks,
Emil

[1] https://lists.freedesktop.org/archives/mesa-dev/2018-February/185822.html
[2] Must admit the last one, isn't at 100% quite yet.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/5] nouveau: Add framebuffer modifier support

[Thierry Reding]

On Wed, Feb 21, 2018 at 04:37:45PM +, Emil Velikov wrote:
> Hi Thierry,
> 
> On 21 February 2018 at 15:30, Thierry Reding  wrote:
> 
> >
> >  struct pipe_resource *
> >  nouveau_buffer_create(struct pipe_screen *pscreen,
> > -  const struct pipe_resource *templ);
> > +  const struct pipe_resource *templ,
> > +  const uint64_t *modifiers, unsigned int count);
> >
> The extra arguments seem unused. I guess it's a left-over from earlier
> iteration?
> Or perhaps you have extra patches that depend on this?

I don't exactly recall why I added those. I guess I must've thought that
the call to nouveau_buffer_create() should be symmetric with the call to
nvc0_miptree_create().

But you're right, I don't think it makes sense to have modifiers for
PIPE_BUFFER, so I'll drop these.

> 
> 
> >  struct pipe_resource *
> >  nouveau_user_buffer_create(struct pipe_screen *screen, void *ptr,
> > diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c 
> > b/src/gallium/drivers/nouveau/nouveau_screen.c
> > index c144b39b2dd2..d651cc7f4b8c 100644
> > --- a/src/gallium/drivers/nouveau/nouveau_screen.c
> > +++ b/src/gallium/drivers/nouveau/nouveau_screen.c
> > @@ -1,3 +1,5 @@
> > +#include 
> > +
> >  #include "pipe/p_defines.h"
> >  #include "pipe/p_screen.h"
> >  #include "pipe/p_state.h"
> > @@ -23,6 +25,8 @@
> >  #include "nouveau_mm.h"
> >  #include "nouveau_buffer.h"
> >
> > +#include "nvc0/nvc0_resource.h"
> > +
> > +static uint64_t nouveau_bo_get_modifier(struct nouveau_bo *bo)
> > +{
> > +   struct nouveau_device *dev = bo->device;
> > +
> > +   if (dev->chipset >= 0xc0)
> > +  return nvc0_bo_get_modifier(bo);
> > +
> > +   return DRM_FORMAT_MOD_INVALID;
> > +}
> >
> Normally the backends (include and) call into the core nouveau code.
> Calling In the opposite direction is achieved via vfuncs, IIRC.

I think I've figured out a much nicer way to fix this (see also my reply
to Ilia's comment). I'll follow up with a patch to show what I mean.

> 
> 
> > switch (templ->target) {
> > case PIPE_BUFFER:
> > -  return nouveau_buffer_create(screen, templ);
> > +  return nouveau_buffer_create(screen, templ, &modifier, 1);
> > default:
> >return nv50_miptree_create(screen, templ);
> > }
> > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c 
> > b/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c
> > index 27674f72a7c0..627d6b7346c3 100644
> > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c
> > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c
> > @@ -20,6 +20,8 @@
> >   * OTHER DEALINGS IN THE SOFTWARE.
> >   */
> >
> > +#include 
> > +
> >  #include "pipe/p_state.h"
> >  #include "pipe/p_defines.h"
> >  #include "util/u_inlines.h"
> > @@ -244,7 +246,8 @@ const struct u_resource_vtbl nvc0_miptree_vtbl =
> >
> >  struct pipe_resource *
> >  nvc0_miptree_create(struct pipe_screen *pscreen,
> > -const struct pipe_resource *templ)
> > +const struct pipe_resource *templ,
> > +const uint64_t *modifiers, unsigned int count)
> >  {
> > struct nouveau_device *dev = nouveau_screen(pscreen)->device;
> > struct nouveau_drm *drm = nouveau_screen(pscreen)->drm;
> > @@ -277,6 +280,9 @@ nvc0_miptree_create(struct pipe_screen *pscreen,
> >}
> > }
> >
> > +   if (count == 1 && modifiers[0] == DRM_FORMAT_MOD_LINEAR)
> > +  pt->flags |= NOUVEAU_RESOURCE_FLAG_LINEAR;
> > +
> Through the patch count 1 + DRM_FORMAT_MOD_INVALID is used, yet
> DRM_FORMAT_MOD_LINEAR is checked above.
> Am I having a silly moment and those should be the same or ?

count == 1 and DRM_FORMAT_MOD_INVALID is used in those cases where we
don't care about modifiers.

In this case, however, the idea is that if we're passed in a single
modifier that happens to be DRM_FORMAT_MOD_LINEAR, we do want to make
sure that we're getting a pitch-linear buffer because it's been
requested.

Note that this is a somewhat minimal way of dealing with modifiers. To
be correct we'd have to pass along exactly the modifiers that we got in
the list. For example a caller could pass a list of block-linear
modifiers with different block heights each, in order of priority. That
is something which we would then have to use to override the values
chosen by nvc0_tex_choose_tile_dims_helper().

In practice, however, we don't really care. Typically we'll just run
with whatever Nouveau has determined to be the best tile_mode. For any
2D texture we should be able to deal with it in the display engine. So
I don't really forsee anyone passing in a specific block height, but
either only DRM_FORMAT_MOD_LINEAR (if they want to render to it using
the CPU, for example) or DRM_FORMAT_MOD_INVALID (don't care, use what
Nouveau thinks is best).

> 
> > +static void
> > +nvc0_query_dmabuf_modifiers(struct pipe_screen *screen,
> > +enum pipe_format format, int max,
> > +  

Re: [Mesa-dev] [PATCH 4/5] nouveau: Add framebuffer modifier support

[Thierry Reding]

On Wed, Feb 21, 2018 at 11:05:45AM -0500, Ilia Mirkin wrote:
> On Wed, Feb 21, 2018 at 10:30 AM, Thierry Reding
>  wrote:
> > From: Thierry Reding 
> >
> > This adds support for framebuffer modifiers to Nouveau. This will be
> > used by the Tegra driver to share metadata about the format of buffers
> > (such as the tiling mode or compression).
> >
> > Signed-off-by: Thierry Reding 
> > ---
> >  src/gallium/drivers/nouveau/Android.mk   |  3 +
> >  src/gallium/drivers/nouveau/Makefile.am  |  1 +
> >  src/gallium/drivers/nouveau/nouveau_buffer.c |  3 +-
> >  src/gallium/drivers/nouveau/nouveau_buffer.h |  3 +-
> >  src/gallium/drivers/nouveau/nouveau_screen.c | 14 +
> >  src/gallium/drivers/nouveau/nv30/nv30_resource.c |  6 +-
> >  src/gallium/drivers/nouveau/nv50/nv50_resource.c |  5 +-
> >  src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c  |  8 ++-
> >  src/gallium/drivers/nouveau/nvc0/nvc0_resource.c | 80 
> > +++-
> >  src/gallium/drivers/nouveau/nvc0/nvc0_resource.h |  5 +-
> >  10 files changed, 120 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/gallium/drivers/nouveau/Android.mk 
> > b/src/gallium/drivers/nouveau/Android.mk
> > index 2de22e73ec18..a446774a86e8 100644
> > --- a/src/gallium/drivers/nouveau/Android.mk
> > +++ b/src/gallium/drivers/nouveau/Android.mk
> > @@ -36,6 +36,9 @@ LOCAL_SRC_FILES := \
> > $(NVC0_CODEGEN_SOURCES) \
> > $(NVC0_C_SOURCES)
> >
> > +LOCAL_C_INCLUDES := \
> > +   $(MESA_TOP)/include/drm-uapi
> > +
> >  LOCAL_SHARED_LIBRARIES := libdrm_nouveau
> >  LOCAL_MODULE := libmesa_pipe_nouveau
> >
> > diff --git a/src/gallium/drivers/nouveau/Makefile.am 
> > b/src/gallium/drivers/nouveau/Makefile.am
> > index 91547178e397..f6126b544811 100644
> > --- a/src/gallium/drivers/nouveau/Makefile.am
> > +++ b/src/gallium/drivers/nouveau/Makefile.am
> > @@ -24,6 +24,7 @@ include Makefile.sources
> >  include $(top_srcdir)/src/gallium/Automake.inc
> >
> >  AM_CPPFLAGS = \
> > +   -I$(top_srcdir)/include/drm-uapi \
> > $(GALLIUM_DRIVER_CFLAGS) \
> > $(LIBDRM_CFLAGS) \
> > $(NOUVEAU_CFLAGS)
> > diff --git a/src/gallium/drivers/nouveau/nouveau_buffer.c 
> > b/src/gallium/drivers/nouveau/nouveau_buffer.c
> > index 2c604419ce05..73afff961115 100644
> > --- a/src/gallium/drivers/nouveau/nouveau_buffer.c
> > +++ b/src/gallium/drivers/nouveau/nouveau_buffer.c
> > @@ -636,7 +636,8 @@ const struct u_resource_vtbl nouveau_buffer_vtbl =
> >
> >  struct pipe_resource *
> >  nouveau_buffer_create(struct pipe_screen *pscreen,
> > -  const struct pipe_resource *templ)
> > +  const struct pipe_resource *templ,
> > +  const uint64_t *modifiers, unsigned int count)
> >  {
> > struct nouveau_screen *screen = nouveau_screen(pscreen);
> > struct nv04_resource *buffer;
> > diff --git a/src/gallium/drivers/nouveau/nouveau_buffer.h 
> > b/src/gallium/drivers/nouveau/nouveau_buffer.h
> > index 3a33fae9ce2f..466f8cc2b466 100644
> > --- a/src/gallium/drivers/nouveau/nouveau_buffer.h
> > +++ b/src/gallium/drivers/nouveau/nouveau_buffer.h
> > @@ -89,7 +89,8 @@ nouveau_resource_mapped_by_gpu(struct pipe_resource 
> > *resource)
> >
> >  struct pipe_resource *
> >  nouveau_buffer_create(struct pipe_screen *pscreen,
> > -  const struct pipe_resource *templ);
> > +  const struct pipe_resource *templ,
> > +  const uint64_t *modifiers, unsigned int count);
> >
> >  struct pipe_resource *
> >  nouveau_user_buffer_create(struct pipe_screen *screen, void *ptr,
> > diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c 
> > b/src/gallium/drivers/nouveau/nouveau_screen.c
> > index c144b39b2dd2..d651cc7f4b8c 100644
> > --- a/src/gallium/drivers/nouveau/nouveau_screen.c
> > +++ b/src/gallium/drivers/nouveau/nouveau_screen.c
> > @@ -1,3 +1,5 @@
> > +#include 
> > +
> >  #include "pipe/p_defines.h"
> >  #include "pipe/p_screen.h"
> >  #include "pipe/p_state.h"
> > @@ -23,6 +25,8 @@
> >  #include "nouveau_mm.h"
> >  #include "nouveau_buffer.h"
> >
> > +#include "nvc0/nvc0_resource.h"
> > +
> >  /* XXX this should go away */
> >  #include "state_tracker/drm_driver.h"
> >
> > @@ -124,6 +128,15 @@ nouveau_screen_bo_from_handle(struct pipe_screen 
> > *pscreen,
> > return bo;
> >  }
> >
> > +static uint64_t nouveau_bo_get_modifier(struct nouveau_bo *bo)
> > +{
> > +   struct nouveau_device *dev = bo->device;
> > +
> > +   if (dev->chipset >= 0xc0)
> > +  return nvc0_bo_get_modifier(bo);
> > +
> > +   return DRM_FORMAT_MOD_INVALID;
> > +}
> >
> >  bool
> >  nouveau_screen_bo_get_handle(struct pipe_screen *pscreen,
> > @@ -131,6 +144,7 @@ nouveau_screen_bo_get_handle(struct pipe_screen 
> > *pscreen,
> >   unsigned stride,
> >   struct winsys_handle *whandle)
> >  {
> > +   whandle->modifier = nouveau_bo_get_modifier(bo);
> > whandle->stride

[Mesa-dev] [Bug 105171] performance regression (3x slower) running glamor with PutImage workload (radeonsi)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105171

Clemens Eisserer  changed:

   What|Removed |Added

 QA Contact|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop.
   |.org|org
   Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop.
   |.org|org
Summary|Performance regression  |performance regression (3x
   |running glamor with |slower) running glamor with
   |PutImage workload   |PutImage workload
   ||(radeonsi)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Emil Velikov]

On 22 February 2018 at 05:57, Tomasz Figa  wrote:
> On Thu, Feb 22, 2018 at 7:23 AM, Rob Herring  wrote:
>>
>> On Wed, Feb 21, 2018 at 1:22 PM, Emil Velikov  
>> wrote:
>> > On 21 February 2018 at 18:50, Rob Herring  wrote:
>> >> On Wed, Feb 21, 2018 at 10:01 AM, Emil Velikov  
>> >> wrote:
>> >>> Hi all,
>> >>>
>> >>> Pardon for dropping in late. I think you've got nearly everything
>> >>> settled down, just sharing a couple of ideas.
>> >>>
>> >>> On 21 February 2018 at 04:19, Tomasz Figa  wrote:
>>  On Wed, Feb 21, 2018 at 4:03 AM, Rob Herring  wrote:
>> > On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa  
>> > wrote:
>> >>>
>>  It is actually incorrect to have the same device FD used for different
>>  screens, if PRIME is used, due to GEM namespace issues, e.g.
>>  - screen 0: drmPrimeFdToHandle(buffer A) -> 1,
>>  - screen 1: drmPrimeFdToHandle(buffer A) -> 1 (same handle in same
>>  namespace, due to semantics of PRIME import and same device FD used),
>>  - screen 0: GEM_CLOSE(handle = 1),
>>  - screen 1 still thinks handle 1 is valid...
>> 
>>  So this only works for control nodes using flink only. (Or if you
>>  always use libdrm_* for BO management and your particular
>>  implementation does its own GEM handle reference counting, but that's
>>  not guaranted.)
>> 
>> >>> Had a sneaky feeling that that != 1 will be returned for screen 1's
>> >>> drmPrimeFdToHandle call.
>> >>> Regardless, moving to DRI3/dmabuf-only setups is the [really] long
>> >>> term plan, I think.
>> >>> I don't know if we can achieve it outside of CrOS - with all the
>> >>> distros building and shipping things in subtly different manner.
>> >>
>> >> It's already possible for Android.
>> >>
>> > Great. Feel free to join me in pushing distributions forward ;-)
>>
>> I'll trade you Android for any thing else. :)
>>
>>
>> > How would that work if you support different GPUs in one image?
>> 
>>  It wouldn't and that's why I prefer probing available devices.
>> 
>>  For Chrome OS, we don't include GPU bits in system image, but rather
>>  have vendor images built separately for each board (although sharing
>>  any possible contents across board family, chipset, GPU type and so
>>  on), so we can have a custom .rc script (which sets a property) as
>>  well. It would even be possible to have paths hardcoded, but that
>>  would be prone to probe ordering issues which, with software fallback,
>>  could be actually not obvious to notice, and so we'd rather not go
>>  this route.
>> 
>>  So I'd agree here that we should revisit probing.
>> 
>> >>> As you pointed out the fallback is not a good idea.
>> >>
>> >> Software fallback is a desired feature. Basing it on the path is the bad 
>> >> idea.
>> >>
>> > Even considering that a) the transition is not obvious and b) it will
>> > cause serve performance degradation?
>> > For development and prototyping purposes it's great. For production
>> > ... very few users will like the abysmal performance :-\
>>
>> Better slow than not booting. There are also usecases such as running
>> in the cloud for running tests where performance is not too important.
>> AOSP has "devices" now that build images for GCE. CrOS does something
>> similar from what Tomasz has said.
>>
>> Software rendering should be the easy case, but some reason is not.
>
> I think software fallback can be done reasonably right. Perhaps it
> could kick in by default only if there is no matching hardware driver?
> If there is a matching driver, but initialization fails, perhaps it
> could make sense to boil out.
>
An explicit fallback or using SW on cloud-like services is perfectly reasonable.
FWIW there is an EGL API for explicit device selection, Mesa
implementation is WIP tough.

>>
>> >>> Plus, as the drm
>> >>> node vary, one can use an Android property and match it to the info.
>> >>> from drmGetDevice2.
>> >>> Say the HW bus location, (sub)device PCIID, other.
>> >>
>> >> That generally doesn't work for non-x86.
>> >>
>> > Errr... wrong. If PCI bus does not exist (some arm boards to have them
>> > though), one can use usb, platform or host1x specifics.
>> > If those are not enough, suggestions are welcome.
>>
>> How does this work across platforms? Feels like just punting the
>> problem to someone else. Currently it's in gralloc, so let's move it
>> to init to set a property? Also, I'm not sure that setting a property
>> is going to work in the future with Treble.
>>
>> But suppose we do make it a property. Does it really matter that the
>> property be a h/w device id and not be the device path? Whatever sets
>> the property can figure out the path based on bus info or whatever
>> logic it wants. And then for my simple uses, that logic can just be
>> hardcoded (as we have today).
>
> Well, we could have property that forces the probing logic to only
> match requested driver or bus ID. If no property is set, i

[Mesa-dev] [Bug 105207] The Talos Principle freezes system using radv

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105207

Bug ID: 105207
   Summary: The Talos Principle freezes system using radv
   Product: Mesa
   Version: unspecified
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Vulkan/radeon
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: pritzl3...@gmail.com
QA Contact: mesa-dev@lists.freedesktop.org

Created attachment 137529
  --> https://bugs.freedesktop.org/attachment.cgi?id=137529&action=edit
dmesg

When playing The Talos principle at 3840*2160 and using "Max 3D Rendering MPIX"
set to 8.4 (4K 2160) the game frequently hangs the system. If I lower the
setting to 1.8 (1680x1050) the game does not seem to hang.
This has happened for as long as I can remember I just have not filed a bug
until now.

I can ssh to the system after this happens but I can not reboot the system.

I am using a Vega 64, linux 4.16-rc2 and Mesa 18.0.0-rc4. Please let me know if
I can provide any logs that could help

Attaching dmesg after the hang has happened.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] ac/nir: set GLC=1 for load/store of coherent/volatile images

[Samuel Pitoiset]

On 02/22/2018 11:45 AM, Bas Nieuwenhuizen wrote:

Ideally we find a way to also do it for buffers, but for this series


I didn't mention it in the commit message, but yes we should.



Reviewed-by: Bas Nieuwenhuizen >


On Thu, Feb 22, 2018 at 10:25 AM, Samuel Pitoiset 
mailto:samuel.pitoi...@gmail.com>> wrote:


This disables persistence accross wavefronts.

F1 2017 and Wolfenstein 2 appear to use some coherent images
but this patch doesn't seem to change anything.

Signed-off-by: Samuel Pitoiset mailto:samuel.pitoi...@gmail.com>>
---
  src/amd/common/ac_nir_to_llvm.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c
b/src/amd/common/ac_nir_to_llvm.c
index 213306322d..99a3471305 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -3657,13 +3657,13 @@ static LLVMValueRef visit_image_load(struct
ac_nir_context *ctx,
                 res = ac_to_integer(&ctx->ac, res);
         } else {
                 LLVMValueRef da = glsl_is_array_image(type) ?
ctx->ac.i1true : ctx->ac.i1false;
-               LLVMValueRef glc = ctx->ac.i1false;
                 LLVMValueRef slc = ctx->ac.i1false;

                 params[0] = get_image_coords(ctx, instr);
                 params[1] = get_sampler_desc(ctx,
instr->variables[0], AC_DESC_IMAGE, NULL, true, false);
                 params[2] = LLVMConstInt(ctx->ac.i32, 15, false);
/* dmask */
-               params[3] = glc;
+               params[3] = (var->data.image._volatile ||
var->data.image.coherent) ?
+                           ctx->ac.i1true : ctx->ac.i1false;
                 params[4] = slc;
                 params[5] = ctx->ac.i1false;
                 params[6] = da;
@@ -3711,7 +3711,8 @@ static void visit_image_store(struct
ac_nir_context *ctx,
                 params[1] = get_image_coords(ctx, instr); /* coords */
                 params[2] = get_sampler_desc(ctx,
instr->variables[0], AC_DESC_IMAGE, NULL, true, true);
                 params[3] = LLVMConstInt(ctx->ac.i32, 15, false);
/* dmask */
-               params[4] = glc;
+               params[4] = (force_glc || var->data.image._volatile
|| var->data.image.coherent) ?
+                           ctx->ac.i1true : ctx->ac.i1false;
                 params[5] = slc;
                 params[6] = ctx->ac.i1false;
                 params[7] = da;
--
2.16.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org 
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] ac/nir: set GLC=1 for load/store of coherent/volatile images

[Bas Nieuwenhuizen]

Ideally we find a way to also do it for buffers, but for this series

Reviewed-by: Bas Nieuwenhuizen 

On Thu, Feb 22, 2018 at 10:25 AM, Samuel Pitoiset  wrote:

> This disables persistence accross wavefronts.
>
> F1 2017 and Wolfenstein 2 appear to use some coherent images
> but this patch doesn't seem to change anything.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/common/ac_nir_to_llvm.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_
> llvm.c
> index 213306322d..99a3471305 100644
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -3657,13 +3657,13 @@ static LLVMValueRef visit_image_load(struct
> ac_nir_context *ctx,
> res = ac_to_integer(&ctx->ac, res);
> } else {
> LLVMValueRef da = glsl_is_array_image(type) ?
> ctx->ac.i1true : ctx->ac.i1false;
> -   LLVMValueRef glc = ctx->ac.i1false;
> LLVMValueRef slc = ctx->ac.i1false;
>
> params[0] = get_image_coords(ctx, instr);
> params[1] = get_sampler_desc(ctx, instr->variables[0],
> AC_DESC_IMAGE, NULL, true, false);
> params[2] = LLVMConstInt(ctx->ac.i32, 15, false); /* dmask
> */
> -   params[3] = glc;
> +   params[3] = (var->data.image._volatile ||
> var->data.image.coherent) ?
> +   ctx->ac.i1true : ctx->ac.i1false;
> params[4] = slc;
> params[5] = ctx->ac.i1false;
> params[6] = da;
> @@ -3711,7 +3711,8 @@ static void visit_image_store(struct ac_nir_context
> *ctx,
> params[1] = get_image_coords(ctx, instr); /* coords */
> params[2] = get_sampler_desc(ctx, instr->variables[0],
> AC_DESC_IMAGE, NULL, true, true);
> params[3] = LLVMConstInt(ctx->ac.i32, 15, false); /* dmask
> */
> -   params[4] = glc;
> +   params[4] = (force_glc || var->data.image._volatile ||
> var->data.image.coherent) ?
> +   ctx->ac.i1true : ctx->ac.i1false;
> params[5] = slc;
> params[6] = ctx->ac.i1false;
> params[7] = da;
> --
> 2.16.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 09/21] clover: Track flags per module section

[Pierre Moreau]

On 2018-02-21 — 20:04, Aaron Watry wrote:
> On Wed, Feb 21, 2018 at 4:50 PM, Pierre Moreau  wrote:
> > One flag that needs to be tracked is whether a library is allowed to
> > received mathematics optimisations or not, as the authorisation is given
> > when creating the library while the optimisations are specified when
> > creating the executable.
> >
> > Signed-off-by: Pierre Moreau 
> > ---
> >  src/gallium/state_trackers/clover/core/module.cpp  |  1 +
> >  src/gallium/state_trackers/clover/core/module.hpp  | 13 
> > +
> >  src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp |  3 ++-
> >  src/gallium/state_trackers/clover/llvm/codegen/common.cpp  |  2 +-
> >  src/gallium/state_trackers/clover/tgsi/compiler.cpp|  3 ++-
> >  5 files changed, 15 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/core/module.cpp 
> > b/src/gallium/state_trackers/clover/core/module.cpp
> > index a6c5b98d8e..0e11506d0d 100644
> > --- a/src/gallium/state_trackers/clover/core/module.cpp
> > +++ b/src/gallium/state_trackers/clover/core/module.cpp
> > @@ -163,6 +163,7 @@ namespace {
> >proc(S &s, QT &x) {
> >   _proc(s, x.id);
> >   _proc(s, x.type);
> > + _proc(s, x.flags);
> >   _proc(s, x.size);
> >   _proc(s, x.data);
> >}
> > diff --git a/src/gallium/state_trackers/clover/core/module.hpp 
> > b/src/gallium/state_trackers/clover/core/module.hpp
> > index 2ddd26426f..ff7e9b6234 100644
> > --- a/src/gallium/state_trackers/clover/core/module.hpp
> > +++ b/src/gallium/state_trackers/clover/core/module.hpp
> > @@ -41,14 +41,19 @@ namespace clover {
> >  data_local,
> >  data_private
> >   };
> > + enum class flags_t {
> > +none,
> > +allow_link_options
> > + };
> >
> > - section(resource_id id, enum type type, size_t size,
> > - const std::vector &data) :
> > - id(id), type(type), size(size), data(data) { }
> > - section() : id(0), type(text_intermediate), size(0), data() { }
> > + section(resource_id id, enum type type, flags_t flags,
> > + size_t size, const std::vector &data) :
> > + id(id), type(type), flags(flags), size(size), data(data) 
> > { }
> > + section() : id(0), type(text_intermediate), flags(flags_t::none), 
> > size(0), data() { }
> >
> >   resource_id id;
> >   type type;
> > + flags_t flags;
> >   size_t size;
> >   std::vector data;
> >};
> > diff --git a/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp 
> > b/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp
> > index 40bb426218..8e9d4c7e85 100644
> > --- a/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp
> > +++ b/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp
> > @@ -84,7 +84,8 @@ clover::llvm::build_module_library(const ::llvm::Module 
> > &mod,
> > enum module::section::type 
> > section_type) {
> > module m;
> > const auto code = emit_code(mod);
> > -   m.secs.emplace_back(0, section_type, code.size(), code);
> > +   m.secs.emplace_back(0, section_type, module::section::flags_t::none,
> > +   code.size(), code);
> > return m;
> >  }
> >
> > diff --git a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp 
> > b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp
> > index ddf2083f37..3a08f11fcc 100644
> > --- a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp
> > +++ b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp
> > @@ -179,7 +179,7 @@ namespace {
> > make_text_section(const std::vector &code) {
> >const pipe_llvm_program_header header { uint32_t(code.size()) };
> >module::section text { 0, module::section::text_executable,
> > - header.num_bytes, {} };
> > + module::section::flags_t::none, 
> > header.num_bytes, {} };
> >
> >text.data.insert(text.data.end(), reinterpret_cast > *>(&header),
> > reinterpret_cast(&header) + 
> > sizeof(header));
> > diff --git a/src/gallium/state_trackers/clover/tgsi/compiler.cpp 
> > b/src/gallium/state_trackers/clover/tgsi/compiler.cpp
> > index e165311fa4..46b8ca7a07 100644
> > --- a/src/gallium/state_trackers/clover/tgsi/compiler.cpp
> > +++ b/src/gallium/state_trackers/clover/tgsi/compiler.cpp
> > @@ -91,7 +91,8 @@ namespace {
> >
> >unsigned sz = tgsi_num_tokens(prog) * sizeof(tgsi_token);
> >std::vector data( (char *)prog, (char *)prog + sz );
> > -  m.secs.push_back({ 0, module::section::text_executable, sz, data });
> > +  m.secs.push_back({ 0, module::section::text_executable,
> > + module::section::flags_t::none, sz, data });
> 
> If you re-order the TGSI-backend removal, this 

Re: [Mesa-dev] [PATCH v3 21/21] clover: Use OpenCL 2.1 defines in place of cl_khr_il_program

[Pierre Moreau]

> I think this can be merged into patches 17 and 18, no?

You are right, will do.

> On Wed, Feb 21, 2018 at 11:50 PM, Pierre Moreau  wrote:
> > Signed-off-by: Pierre Moreau 
> > ---
> >  src/gallium/state_trackers/clover/api/device.cpp  | 3 +--
> >  src/gallium/state_trackers/clover/api/program.cpp | 2 +-
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/api/device.cpp 
> > b/src/gallium/state_trackers/clover/api/device.cpp
> > index 9132044b9c..9c19694c20 100644
> > --- a/src/gallium/state_trackers/clover/api/device.cpp
> > +++ b/src/gallium/state_trackers/clover/api/device.cpp
> > @@ -333,11 +333,10 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info 
> > param,
> >buf.as_string() = dev.supported_extensions();
> >break;
> >
> > -   case CL_DEVICE_IL_VERSION_KHR:
> > +   case CL_DEVICE_IL_VERSION:
> >if (dev.supported_extensions().find("cl_khr_il_program") == 
> > std::string::npos)
> >   throw error(CL_INVALID_VALUE);
> >buf.as_string() = std::string("SPIR-V_1.0");
> > -
> >break;
> >
> > case CL_DEVICE_PLATFORM:
> > diff --git a/src/gallium/state_trackers/clover/api/program.cpp 
> > b/src/gallium/state_trackers/clover/api/program.cpp
> > index c920168250..0dea2f81fa 100644
> > --- a/src/gallium/state_trackers/clover/api/program.cpp
> > +++ b/src/gallium/state_trackers/clover/api/program.cpp
> > @@ -460,7 +460,7 @@ clGetProgramInfo(cl_program d_prog, cl_program_info 
> > param,
> >buf.as_string() = prog.source();
> >break;
> >
> > -   case CL_PROGRAM_IL_KHR:
> > +   case CL_PROGRAM_IL:
> >if (prog.has_il)
> >   buf.as_vector() = prog.il();
> >else if (r_size)
> > --
> > 2.16.2
> >


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 13/21] configure.ac, meson: Check for SPIRV-Tools and llvm-spirv

[Pierre Moreau]

> do we already have an upstream version of both dependencies we could
> just use? Or do we still need special branches?

For both we can use the master branch of the upstream version. SPIRV-Tools
validator is still not perfect, but it does already catch issues in OpenCL
SPIR-V binaries. And llvm-spirv needs to be “rebased” as some commits went
missing during the update process, but that is not going to impact the API
used; we can absolutely wait for that to finish before merging the series.

Pierre

> On Wed, Feb 21, 2018 at 11:50 PM, Pierre Moreau  wrote:
> > Signed-off-by: Pierre Moreau 
> > ---
> >
> > Notes:
> > v3:
> > * Bump the required version of SPIRV-Tools to the latest release;
> > * Add a dependency on llvm-spirv.
> >
> >  configure.ac | 10 ++
> >  meson.build  |  4 
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 8a9172690a..1d393a5234 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -2386,6 +2386,16 @@ AM_CONDITIONAL(HAVE_CLOVER_ICD, test 
> > "x$enable_opencl_icd" = xyes)
> >  AC_SUBST([OPENCL_LIBNAME])
> >  AC_SUBST([CLANG_RESOURCE_DIR])
> >
> > +AS_IF([test "x$enable_opencl" = xyes], [
> > +PKG_CHECK_MODULES([SPIRV_TOOLS], [SPIRV-Tools >= 2018.0])])
> > +AC_SUBST([SPIRV_TOOLS_CFLAGS])
> > +AC_SUBST([SPIRV_TOOLS_LIBS])
> > +
> > +AS_IF([test "x$enable_opencl" = xyes], [
> > +PKG_CHECK_MODULES([LLVM_SPIRV], [llvm-spirv])])
> > +AC_SUBST([LLVM_SPIRV_CFLAGS])
> > +AC_SUBST([LLVM_SPIRV_LIBS])
> > +
> >  dnl
> >  dnl Gallium configuration
> >  dnl
> > diff --git a/meson.build b/meson.build
> > index 8cf67b8171..c67bd32d0f 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -596,10 +596,14 @@ if _opencl != 'disabled'
> >
> ># TODO: alitvec?
> >dep_clc = dependency('libclc')
> > +  dep_spirv_tools = dependency('SPIRV-Tools', version : '>= 2018.0')
> > +  dep_llvm_spirv = dependency('llvm-spirv')
> >with_gallium_opencl = true
> >with_opencl_icd = _opencl == 'icd'
> >  else
> >dep_clc = []
> > +  dep_spirv_tools = []
> > +  dep_llvm_spirv = []
> >with_gallium_opencl = false
> >with_gallium_icd = false
> >  endif
> > --
> > 2.16.2
> >


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 21/21] clover: Use OpenCL 2.1 defines in place of cl_khr_il_program

[Karol Herbst]

I think this can be merged into patches 17 and 18, no?

On Wed, Feb 21, 2018 at 11:50 PM, Pierre Moreau  wrote:
> Signed-off-by: Pierre Moreau 
> ---
>  src/gallium/state_trackers/clover/api/device.cpp  | 3 +--
>  src/gallium/state_trackers/clover/api/program.cpp | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/device.cpp 
> b/src/gallium/state_trackers/clover/api/device.cpp
> index 9132044b9c..9c19694c20 100644
> --- a/src/gallium/state_trackers/clover/api/device.cpp
> +++ b/src/gallium/state_trackers/clover/api/device.cpp
> @@ -333,11 +333,10 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info 
> param,
>buf.as_string() = dev.supported_extensions();
>break;
>
> -   case CL_DEVICE_IL_VERSION_KHR:
> +   case CL_DEVICE_IL_VERSION:
>if (dev.supported_extensions().find("cl_khr_il_program") == 
> std::string::npos)
>   throw error(CL_INVALID_VALUE);
>buf.as_string() = std::string("SPIR-V_1.0");
> -
>break;
>
> case CL_DEVICE_PLATFORM:
> diff --git a/src/gallium/state_trackers/clover/api/program.cpp 
> b/src/gallium/state_trackers/clover/api/program.cpp
> index c920168250..0dea2f81fa 100644
> --- a/src/gallium/state_trackers/clover/api/program.cpp
> +++ b/src/gallium/state_trackers/clover/api/program.cpp
> @@ -460,7 +460,7 @@ clGetProgramInfo(cl_program d_prog, cl_program_info param,
>buf.as_string() = prog.source();
>break;
>
> -   case CL_PROGRAM_IL_KHR:
> +   case CL_PROGRAM_IL:
>if (prog.has_il)
>   buf.as_vector() = prog.il();
>else if (r_size)
> --
> 2.16.2
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 13/21] configure.ac, meson: Check for SPIRV-Tools and llvm-spirv

[Karol Herbst]

do we already have an upstream version of both dependencies we could
just use? Or do we still need special branches?

On Wed, Feb 21, 2018 at 11:50 PM, Pierre Moreau  wrote:
> Signed-off-by: Pierre Moreau 
> ---
>
> Notes:
> v3:
> * Bump the required version of SPIRV-Tools to the latest release;
> * Add a dependency on llvm-spirv.
>
>  configure.ac | 10 ++
>  meson.build  |  4 
>  2 files changed, 14 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index 8a9172690a..1d393a5234 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2386,6 +2386,16 @@ AM_CONDITIONAL(HAVE_CLOVER_ICD, test 
> "x$enable_opencl_icd" = xyes)
>  AC_SUBST([OPENCL_LIBNAME])
>  AC_SUBST([CLANG_RESOURCE_DIR])
>
> +AS_IF([test "x$enable_opencl" = xyes], [
> +PKG_CHECK_MODULES([SPIRV_TOOLS], [SPIRV-Tools >= 2018.0])])
> +AC_SUBST([SPIRV_TOOLS_CFLAGS])
> +AC_SUBST([SPIRV_TOOLS_LIBS])
> +
> +AS_IF([test "x$enable_opencl" = xyes], [
> +PKG_CHECK_MODULES([LLVM_SPIRV], [llvm-spirv])])
> +AC_SUBST([LLVM_SPIRV_CFLAGS])
> +AC_SUBST([LLVM_SPIRV_LIBS])
> +
>  dnl
>  dnl Gallium configuration
>  dnl
> diff --git a/meson.build b/meson.build
> index 8cf67b8171..c67bd32d0f 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -596,10 +596,14 @@ if _opencl != 'disabled'
>
># TODO: alitvec?
>dep_clc = dependency('libclc')
> +  dep_spirv_tools = dependency('SPIRV-Tools', version : '>= 2018.0')
> +  dep_llvm_spirv = dependency('llvm-spirv')
>with_gallium_opencl = true
>with_opencl_icd = _opencl == 'icd'
>  else
>dep_clc = []
> +  dep_spirv_tools = []
> +  dep_llvm_spirv = []
>with_gallium_opencl = false
>with_gallium_icd = false
>  endif
> --
> 2.16.2
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] vulkan/wsi/wayland: Add support for zwp_dmabuf

[Daniel Stone]

Hi Dieter,

2018-02-22 0:57 GMT+00:00 Dieter Nützel :
> Making all in vulkan
> make[3]: Verzeichnis „/opt/mesa/src/vulkan“ wird betreten
> make[3]: *** Keine Regel vorhanden, um das Ziel
> „/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml“,
>   benötigt von „wsi/linux-dmabuf-unstable-v1-protocol.c“, zu erstellen.
> Schluss.
> make[3]: Verzeichnis „/opt/mesa/src/vulkan“ wird verlassen
> make[2]: *** [Makefile:862: all-recursive] Fehler 1
> make[2]: Verzeichnis „/opt/mesa/src“ wird verlassen
> make[1]: *** [Makefile:653: all] Fehler 2
> make[1]: Verzeichnis „/opt/mesa/src“ wird verlassen
> make: *** [Makefile:666: all-recursive] Fehler 1
>
> I do not have (do not know of) any wayland stuff running, here.

Thanks for reporting this, and sorry for the brekage. I did test with
a no-Wayland autotool before pushing, but that succeeded for me. To
fix the build, you can either use Meson or just install
wayland-protocols on your system: no need to enable the Wayland build
in Mesa or use it at all, just have it available in $PKG_CONFIG_PATH.
(It's just a pile of XML, no need to compile anything.)

Emil, you said in an earlier discussion[0] that you'd 'rather deal
with issues as they appear': here's one! Vulkan's Wayland WSI now
depends on generating linux-dmabuf protocol. It's unconditionally in
BUILT_SOURCES, I guess to ensure proper ordering. When
wayland-protocols isn't available on the system, the build fails due
to BUILT_SOURCES depending on WL_DMABUF_XML, which does not exist.
wayland-drm was already in BUILT_SOURCES, but that doesn't seem to
cause any issues as the dependent files are there in tree. autotools
seems to want the dependencies of the target there, but doesn't care
about the target itself? Even if you pass
--with-platforms=x11,drm,surfaceless at build, it still succeeds
because the $(WAYLAND_PROTOCOLS_DATADIR) substitution is done
unconditionally at top-level, even if you've requested to not build
the Wayland platform.

FWIW, having forgotten about that patch, I actually typed up what
turned out to be a straight revert of it, to try to eliminate the
inconsistency and make it possible to check this when doing local
builds. To be honest, I still have no idea why we dist the generated
files; as I said at the time, I'd prefer to either have them generated
at build (requires wayland-scanner and wayland-protocols installed,
which is hardly onerous if you're building the Wayland platform), or
just never ever generate them and check pre-generated files into the
tree. The current mix and match seems pretty problematic.

How would you fix this?

Cheers,
Daniel

[0]: https://lists.freedesktop.org/archives/mesa-dev/2017-July/163825.html
  cf. 166852ee957ff25a1d14dcaa7512917343130fa4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] ac/nir: set GLC=1 for load/store of coherent/volatile images

[Samuel Pitoiset]

This disables persistence accross wavefronts.

F1 2017 and Wolfenstein 2 appear to use some coherent images
but this patch doesn't seem to change anything.

Signed-off-by: Samuel Pitoiset 
---
 src/amd/common/ac_nir_to_llvm.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 213306322d..99a3471305 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -3657,13 +3657,13 @@ static LLVMValueRef visit_image_load(struct 
ac_nir_context *ctx,
res = ac_to_integer(&ctx->ac, res);
} else {
LLVMValueRef da = glsl_is_array_image(type) ? ctx->ac.i1true : 
ctx->ac.i1false;
-   LLVMValueRef glc = ctx->ac.i1false;
LLVMValueRef slc = ctx->ac.i1false;
 
params[0] = get_image_coords(ctx, instr);
params[1] = get_sampler_desc(ctx, instr->variables[0], 
AC_DESC_IMAGE, NULL, true, false);
params[2] = LLVMConstInt(ctx->ac.i32, 15, false); /* dmask */
-   params[3] = glc;
+   params[3] = (var->data.image._volatile || 
var->data.image.coherent) ?
+   ctx->ac.i1true : ctx->ac.i1false;
params[4] = slc;
params[5] = ctx->ac.i1false;
params[6] = da;
@@ -3711,7 +3711,8 @@ static void visit_image_store(struct ac_nir_context *ctx,
params[1] = get_image_coords(ctx, instr); /* coords */
params[2] = get_sampler_desc(ctx, instr->variables[0], 
AC_DESC_IMAGE, NULL, true, true);
params[3] = LLVMConstInt(ctx->ac.i32, 15, false); /* dmask */
-   params[4] = glc;
+   params[4] = (force_glc || var->data.image._volatile || 
var->data.image.coherent) ?
+   ctx->ac.i1true : ctx->ac.i1false;
params[5] = slc;
params[6] = ctx->ac.i1false;
params[7] = da;
-- 
2.16.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] spirv: apply memory qualifiers to images

[Samuel Pitoiset]

Signed-off-by: Samuel Pitoiset 
---
 src/compiler/spirv/vtn_variables.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/compiler/spirv/vtn_variables.c 
b/src/compiler/spirv/vtn_variables.c
index ead68b4784..9eb85c24e9 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1332,6 +1332,15 @@ apply_var_decoration(struct vtn_builder *b, nir_variable 
*nir_var,
   nir_var->data.read_only = true;
   nir_var->data.image.read_only = true;
   break;
+   case SpvDecorationRestrict:
+  nir_var->data.image.restrict_flag = true;
+  break;
+   case SpvDecorationVolatile:
+  nir_var->data.image._volatile = true;
+  break;
+   case SpvDecorationCoherent:
+  nir_var->data.image.coherent = true;
+  break;
case SpvDecorationComponent:
   nir_var->data.location_frac = dec->literals[0];
   break;
@@ -1378,10 +1387,7 @@ apply_var_decoration(struct vtn_builder *b, nir_variable 
*nir_var,
case SpvDecorationRowMajor:
case SpvDecorationColMajor:
case SpvDecorationMatrixStride:
-   case SpvDecorationRestrict:
case SpvDecorationAliased:
-   case SpvDecorationVolatile:
-   case SpvDecorationCoherent:
case SpvDecorationUniform:
case SpvDecorationStream:
case SpvDecorationOffset:
-- 
2.16.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Chih-Wei Huang]

2018-02-21 3:03 GMT+08:00 Rob Herring :
>
> Perhaps worth revisiting. Given we've failed to progress at all since
> then may change opinions some. We already have to handle multiple
> opens share the same pipe_screen, so I don't think reusing the fd buys
> us anything.
>
> Maybe we're close to the point of removing the flink name support too.
> The android-x86 folks have been working to get dma-bufs working.
> Chih-Wei, any comments on this?

Ah! Sorry. I didn't catch or understand the details.
Did you mean the attempts to use drm_hwcomposer
in android-x86?
My understanding so far is most x86 GPUs won't work
except some very limited models.
It's due to kernel driver issues which may never be solved.
So we can't drop the flink name support.
Please correct me if I am wrong.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev