l to sort out and bow out of the
discussion at this point as the series seems to have brought additional issues
up that are out-of-scope for what I had in mind with the series (namely
something small-simple to expose the extension and not enforcing the limitation
of the ARB extension).
-Kevin
-
Hi,
> Is that tested?
I have tested it in a simple shader test I made (i.e. not in a dedicated test
suite such as dEQP, piglit or something else). The created assembly is
identical. The specific example is a shader where I replace calls of
beginFragmentShaderOrderingINTEL() with beginInvocatio
Hi,
> Having the same underlying compiler intrinsic and having the same behavior
> are not the same thing. The INTEL extension allows strictly more
> functionality than the ARB extension so it needs even more testing. In
> particular, it allows you to do the barrier in non-uniform control-flow
>
Hi all,
Patch addressing the missing enum warning is here:
https://lists.freedesktop.org/archives/mesa-dev/2018-August/203796.html . Also,
see my reply to Francisco why I think it is better to have a new intrinsic
function for that.
Best Regards,
-Kevin
Hi,
Sighs; I missed that warning on the build since there is so much build output.
I can issue a small patch to handle the missing enum.
-Kevin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa
they should be 3 different
intrinsics with the thinking that if the Nouveau driver adds support for the
ARB/NV extension it will do the IR analysis to do what is needed for the
critical-section style extension.
-Kevin
From: Rogovin, Kevin
Sent: Tuesday, August 28, 2018 7:05 PM
To: mesa-dev
Hi,
On the questions of tests: If people want, I can adapt the test in piglit for
ARB_fragment_shader_interlock to this INTEL one. In general, I have an
app/library that uses the extension and testing of that does definitely work on
i965 (which should be utterly unsurprising since the produced
Hi,
I followed the convention that was already present. The code from
ARB/NV_fragment_shader_interlock had an intrinsic for begin critical section
and an intrinsic for end critical section. I figured that since this extension
has a completely different thinking (i.e. a memory barrier instead of
posted is
better; if that is not the case, then a nasty, hard to track bug will then lurk.
-Kevin
-Original Message-
From: Palli, Tapani
Sent: Friday, March 16, 2018 11:32 AM
To: mesa-dev@lists.freedesktop.org
Cc: ja...@jlekstrand.net; Rogovin, Kevin ; Palli,
Tapani
Subject: [PATCH v2
.
-Kevin
From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Tuesday, March 6, 2018 6:00 PM
To: Rogovin, Kevin
Cc: ML mesa-dev
Subject: Re: [Mesa-dev] [PATCH v3 2/7] i965: restore diable_aux argument to
intel_miptree_prepare_texture()
We took it out with good reason... I'd rather
passed down along
the function calls somehow …
-Kevin
From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Tuesday, March 6, 2018 6:07 PM
To: Rogovin, Kevin
Cc: ML mesa-dev
Subject: Re: [Mesa-dev] [PATCH v3 7/7] i965: ASTC5x5 workaround logic for blorp
On Tue, Feb 27, 2018 at 1:30 AM
Originally, I had the entire astc and aux checking in a dedicated function;
would that be more preferable?
-Kevin
From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Tuesday, March 6, 2018 6:02 PM
To: Rogovin, Kevin
Cc: ML mesa-dev
Subject: Re: [Mesa-dev] [PATCH v3 3/7] i965: set ASTC5x5
ssage-
From: Palli, Tapani
Sent: Wednesday, February 14, 2018 9:58 AM
To: Rogovin, Kevin ; Jason Ekstrand
Cc: ML mesa-dev
Subject: Re: [Mesa-dev] [PATCH v2 0/5] i965: ASTC5x5 workaround
On 14.02.2018 09:54, Tapani Pälli wrote:
>
>
> On 14.02.2018 09:38, Rogovin, Kevi
Reviewed by: kevin.rogovin [at] intel.com
-Original Message-
From: Phillips, Scott D
Sent: Friday, February 9, 2018 3:11 AM
To: mesa-dev@lists.freedesktop.org; Rogovin, Kevin
Subject: [PATCH v3] intel/tools: new intel_sanitize_gpu tool
From: Kevin Rogovin
Adds a new debug tool to pad
ly in brw_draw.c for resolving
inputs).
-Kevin
-Original Message-
From: Palli, Tapani
Sent: Monday, February 12, 2018 10:14 AM
To: Rogovin, Kevin ; Jason Ekstrand
Cc: ML mesa-dev
Subject: Re: [Mesa-dev] [PATCH v2 0/5] i965: ASTC5x5 workaround
On 02/12/2018 09:44 AM, Tapani Pälli wrote:
HI,
Review comments below.
-Original Message-
From: Phillips, Scott D
Sent: Thursday, February 8, 2018 2:19 AM
To: mesa-dev@lists.freedesktop.org; Rogovin, Kevin
Subject: [PATCH v2] intel/tools: new intel_sanitize_gpu tool
From: Kevin Rogovin
Adds a new debug tool to pad each GEM
.
-Kevin
From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Thursday, February 8, 2018 2:47 AM
To: Rogovin, Kevin
Cc: ML mesa-dev
Subject: Re: [Mesa-dev] [PATCH v2 0/5] i965: ASTC5x5 workaround
Random thought:
Nanley and I were talking about this just now and I was complaining about how
or if the application is using OpenGL and OpenCL (or
libva) ).
2. I would also handle the destroy GEM ioctl to remove its entry from the map.
-Kevin
-Original Message-
From: Phillips, Scott D
Sent: Wednesday, February 7, 2018 2:35 AM
To: mesa-dev@lists.freedesktop.org; Rogovin,
bruary 7, 2018 2:35 AM
To: mesa-dev@lists.freedesktop.org; Rogovin, Kevin
Subject: [PATCH RFC] intel/tools: new intel_sanitize_gpu tool
From: Kevin Rogovin
Adds a new debug tool to pad each GEM BO allocated with (weak) pseudo-random
noise values which are then checked after each batchbuffer dispatc
, 2018 6:41 PM
To: Rogovin, Kevin
Subject: RE: [PATCH v3 3/3] i965: if DEBUG_OUT_OF_BOUND_CHK is up, check that
noise padding for each bo used in batchbuffer is correct
Nope. That one looked fine as-is.
On January 28, 2018 23:13:40 "Rogovin, Kevin" wrote:
> Any comments/revie
Sure, I can change the bit flag name to DEBUG_CHECK_OOB;
From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Friday, January 26, 2018 12:11 PM
To: Rogovin, Kevin
Cc: ML mesa-dev
Subject: Re: [Mesa-dev] [PATCH v3 1/3] intel/common:add debug flag for adding
and checking padding on BO'
padding. This is following the
review comments from Chris Wilson.
-Kevin
From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Friday, January 26, 2018 12:13 PM
To: Rogovin, Kevin
Cc: ML mesa-dev
Subject: Re: [Mesa-dev] [PATCH v3 2/3] i965: add noise padding to buffer object
and function to
: Friday, December 15, 2017 8:34 PM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH v2 0/5] i965: ASTC5x5 workaround
On Thu, Dec 14, 2017 at 07:39:46PM +0200, kevin.rogo...@intel.com wrote:
> From: Kevin Rogovin
>
> This patch series implements a needed w
Hi,
> Yes. It's just the accidental writes into the read-only bo that you may miss.
> Your call, and I have no
> objections if such limitations are described in the comments.
I'll add the comment; since there then less code to read (indeed using
brw_bo_map will not work because
it does not map
up to padding size), I think pread would be fine then.
Thoughts?
-Kevin
P.S. I am already writing it to do mapping and such, but it is more code sadly.
-Original Message-
From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
Sent: Wednesday, December 13, 2017 1:23 PM
To: Rogovin, Kevin ; mes
Hi,
> Actually that's not strictly true. Since you only do a pread here, it will
> only synchronize against the last declared write to the bo.
> There's no guaranteed sync with the last batch for a set of read-only bo.
> Similarly, because of no domain tracking, it won't also ensure that the bo i
Hi,
Just got confirmation that kernel does the syncing required to make sure that
pread values are realiable.
-Kevin
-Original Message-
From: Rogovin, Kevin
Sent: Wednesday, December 13, 2017 8:19 AM
To: 'Jason Ekstrand'
Cc: mesa-dev@lists.freedesktop.org; Lahtinen, Joona
Hi,
> I think you want to do this at the end of submit_batch instead and add a
> brw_bo_wait_rendering on the batch.
> Otherwise, your bounds checking is racing with the GPU.
I remember being told that pread has the kernel do the required waiting,
however I am not 100% sure of this (which is
Hi,
Thankyou for reading the code and giving advice to improve upon it. Below are
some thoughts:
> I can't help but think that this could be a bit simpler and involve throwing
> fewer pointers around.
I was thinking this too; the easiest way to do this is to just have the same
noise for all
t reviewed and land in Mesa so that
hunting for a certain classes of Heisenberg bugs can be less Heisenberg like.
Best Regards,
-Kevin
-Original Message-
From: Kenneth Graunke [mailto:kenn...@whitecape.org]
Sent: Tuesday, December 12, 2017 9:09 PM
To: mesa-dev@lists.freedesktop.org
C
Ahh futz this was the wrong one!! it should just be
subslices += 4 * brw->screen->devinfo.num_slices;
Sighs (too many terminals open with too many branches :P )
I need to post a v2; I will post it shortly.
-Kevin
-Original Message-
From: Rogovin, Kevin
Sent: Tuesday, Decem
> These patches fix GPU hangs I'm seeing also with the *GL* version of
> CarChase on KBL GT3e, when using Ubuntu 16.04 kernel.
> NOTE: those hangs don't happen when doing tests with latest drm-tip
> kernel. Besides the older Ubuntu kernel, I'm seeing hangs also with the
> 4.13 drm-tip kernel,
readability patch, I think it should land to aid
in readability of the code.
-Kevin
-Original Message-
From: Rogovin, Kevin
Sent: Tuesday, December 12, 2017 12:05 PM
To: mesa-dev@lists.freedesktop.org
Cc: Rogovin, Kevin
Subject: [PATCH 1/2] i965: correctly program MEDIA_VFE_STATE for compute
>I'd have to think about it harder but even those are not likely to actually
>get ASTC decode. Maybe for some sort of decompression thing but if you're
>doing
> GetCompressedTexImage, it'll probably turn into a blorp_copy which will use
> R32G32B32A32_UINT.
I am thinking for the case where a
Hi,
>This isn't true. 100% of the intel_mipmap_tree -> blorp_surf translations are
>handled by that function.
> It's a perfectly reasonable place to handle these things. It could also be
> handled in genX(blorp_exec) if that makes someone more comfortable.
This is where I placed the ASTC en
> Are you saying that this bug extends over hardware context?
Different HW contexts imply different execbuffer2 ioctl's. The kernel inserts a
full blown flush of everything after (or before, I cannot remember) each
execbuffer2 call. This way there is context isolation in HW buggineness.
-Kevin
> If you take a look at brw_update_texture_surface(), just in the end before
> brw_emit_surface_state() the logic explictly consults for
> intel_miptree_texture_aux_usage().
> This in turn tells if the auxiliary buffer is resolved and it doesn't need to
> be programmed.
The full stack trace is
Hi,
>> Here are my comments of the patch posted:
>>
>> 1. it is essentially replication and moving around of the code of the
>> patch series posted earlier but missing various
>> important bits: preventing the sampler from using the auxiliary buffer
>> (this requires to modify surface
code level
that only one of the two is possible without texture invalidates.
-Kevin
-Original Message-
From: Topi Pohjolainen [mailto:topi.pohjolai...@gmail.com]
Sent: Monday, December 4, 2017 12:49 PM
To: mesa-dev@lists.freedesktop.org
Cc: Rogovin, Kevin
Subject: [PATCH] RFC: Worka
Hi,
> 1) do extra tex cache flush when needed and specifically only when needed
> 2) resolve surfaces when undesired combination is about to be sampled
>Latter case could be addressed also with on-the-fly check in
>brw_predraw_resolve_inputs(). There one goes thru all the active textures for
>t
--Original Message-
From: Matt Turner [mailto:matts...@gmail.com]
Sent: Friday, December 1, 2017 8:25 PM
To: Rogovin, Kevin
Cc: Ilia Mirkin ; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 0/5] i965: ASTC5x5 workaround
On Fri, Dec 1, 2017 at 10:06 AM, Rogovin, Kevin wrote:
> Hi,
, 2017 7:38 PM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 0/5] i965: ASTC5x5 workaround
On Fri, Dec 1, 2017 at 12:19 PM, wrote:
> From: Kevin Rogovin
>
> This patch series implements a needed workaround for Gen9 for ASTC5x5
> sampler reads.
Thankyou! very much for the patch to the command line disassembler.
-Kevin
-Original Message-
From: Matt Turner [mailto:matts...@gmail.com]
Sent: Friday, November 17, 2017 6:52 AM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 07/18] intel
Thanks, I missed that; sorry for the mailing list chatter caused by missing the
original e-mail on the material.
-Kevin
-Original Message-
From: Eric Engestrom [mailto:eric.engest...@imgtec.com]
Sent: Wednesday, November 15, 2017 1:39 PM
To: Rogovin, Kevin
Cc: mesa-dev
Just to clarify, I do not mean just the defaults, but doing -buildtype release
also leaves assert()'s active (atleast on my system setup).
-Kevin
From: Rogovin, Kevin
Sent: Wednesday, November 15, 2017 11:44 AM
To: mesa-dev@lists.freedesktop.org
Subject: Difference between Meson and auto
Hi,
I do not know if anyone has noticed or if it is deliberate, but the meson
builds have that assert()'s are active but the autotools release builds have
that assert() is inactive.
-Kevin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https
: Matt Turner [mailto:matts...@gmail.com]
Sent: Monday, November 13, 2017 11:21 PM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 07/18] intel/compiler: fix for memmove argument
on annotating error
On Mon, Nov 13, 2017 at 1:12 PM, Rogovin, Kevin wrote:
>
3, 2017 8:09 PM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 18/18] intel/tools: add command line GEN shader
disassembler tool
On Monday, 2017-11-13 15:18:06 +0200, kevin.rogo...@intel.com wrote:
> From: Kevin Rogovin
>
> Signed-off-by: Kevin Rog
Your theory makes sense to me too; I suspect that something in the annotation
code is the ultimate cause.
-Original Message-
From: Matt Turner [mailto:matts...@gmail.com]
Sent: Monday, November 13, 2017 9:15 PM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa
I need this function in order for the logger to save shader binary to disk.
-Kevin
-Original Message-
From: Matt Turner [mailto:matts...@gmail.com]
Sent: Monday, November 13, 2017 9:43 PM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 10/18] intel
...@gmail.com]
Sent: Monday, November 13, 2017 9:42 PM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 08/18] intel/compiler:add function to give
option to print offsets into assembly
On Mon, Nov 13, 2017 at 5:17 AM, wrote:
> From: Kevin Rogovin
>
> S
it
crash on before.
-Kevin
-Original Message-
From: Matt Turner [mailto:matts...@gmail.com]
Sent: Monday, November 13, 2017 9:02 PM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 07/18] intel/compiler: fix for memmove argument
on annotating error
Hi,
Oh MY! I missed that it was added to the driver (because miraculously the patch
applied cleanly anyway).
Thanks, I will drop this patch if I make a v3.
-Kevin
-Original Message-
From: Landwerlin, Lionel G
Sent: Monday, November 13, 2017 3:45 PM
To: Rogovin, Kevin ; mesa-dev
ented fixes for the issue that Chris pointed out
on batchbuffer migration and a pair of issues I realized on the script at patch
17).
Best Regards,
-Kevin
-Original Message-
From: Rogovin, Kevin
Sent: Wednesday, September 27, 2017 2:38 PM
To: Chris Wilson ; mesa-dev@lists.freedeskto
>My worry is that batch->bo is not constant for the construction of a single
>execbuf2.
> If intel_batchbuffer.c runs out of room inside the batch->bo, it will
> allocate a new one and continue building it.
That will be ok, but if the (fd, GEM BO handle) changes, there is a serious
issue.
> I
make such a dedicated tool quite quickly, or add that
functionality to the logger.
-Kevin
-Original Message-
From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
Sent: Wednesday, September 27, 2017 1:21 PM
To: Rogovin, Kevin ; mesa-dev@lists.freedesktop.org
Subject: RE: [Mesa-dev] [PATCH
nel G
Sent: Wednesday, September 27, 2017 12:35 PM
To: Rogovin, Kevin ; Chris Wilson
; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 00/22] RFC: Batchbuffer Logger for Intel GPU
A few months ago I implemented debug messages in the command stream by stuffing
the unused bits of MI
is to send the api-trace call id's to the kernel on
execbuffer2, then this does not matter.
-Kevin
-Original Message-
From: Rogovin, Kevin
Sent: Wednesday, September 27, 2017 9:53 AM
To: 'Chris Wilson' ; mesa-dev@lists.freedesktop.org
Subject: RE: [Mesa-dev] [P
ould leave behind a file).
Let me know, what is best, and I will do it.
-Kevin
-Original Message-
From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
Sent: Tuesday, September 26, 2017 11:20 PM
To: Rogovin, Kevin ; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 00/2
Sent: Tuesday, September 26, 2017 2:53 AM
To: Landwerlin, Lionel G ; Rogovin, Kevin
; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 05/22] i965: correctly assign SamplerCount of
INTERFACE_DESCRIPTOR_DATA
On 2017-09-25 05:46:32, Lionel Landwerlin wrote:
> I'm genuinely surpri
. When the value is zero, it means the GPU will not prefetch the
binding table entries.
-Kevin
-Original Message-
From: Landwerlin, Lionel G
Sent: Monday, September 25, 2017 3:10 PM
To: Rogovin, Kevin ; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 04/22] i965: assign
, August 24, 2017 8:10 PM
To: Rogovin, Kevin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] i965: add 2xMSAA and 16xMSAA to DRI configs for
Gen9.
On 17-08-24 14:16:39, kevin.rogo...@intel.com wrote:
>From: Kevin Rogovin
>
>Special thanks to Eero Tamminen for reporting r
Hello,
> typo -> ...query (same goes for patch 1/2)
I hate typos. Thanks for pointing it out.
> Please correct me if I'm wrong, but I think we cannot unalias functions once
> they're in.
> It will break the backwards compatibility we're trying to manage with glapi.
> If we want to
> retain i
Reviewed-by: Kevin Rogovin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Kevin Rogovin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-Original Message-
From: Rogovin, Kevin
Sent: Wednesday, June 10, 2015 8:56 AM
To: 'Ian Romanick'; mesa-dev@lists.freedesktop.org
Subject: RE: [Mesa-dev] [v4 PATCH 05/10] mesa: helper function for scissor box
of gl_framebuffer
Hi,
-Original Message-
From: Ia
Hi,
-Original Message-
From: Ian Romanick [mailto:i...@freedesktop.org]
Sent: Wednesday, June 10, 2015 1:06 AM
To: Rogovin, Kevin; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [v4 PATCH 05/10] mesa: helper function for scissor box
of gl_framebuffer
>All of the changes
> Out of curiosity, what editor are you using? Usually setting the
> indentation rules in your editor takes care of most of these problems.
I am using emacs23 (under Ubuntu) and now for something amusing...
> We have a .dir-locals.el file that should provide the correct settings
> for Emacs.
e
> You should wait until you get a "real" review from someone before reposting.
I think that is good advice. Indeed, I am not going to post a v5 of this series
until for each patch there is:
Two review bys, possibly qualified with "fix the mentioned formatting issues"
OR
Specific change reques
Hi,
>> +static inline GLuint
>Here and below, why 2 spaces between "inline" and "GLuint"?
I have no clue. I suspect it is a scar from some search/replace fiasco over 3
weeks ago. You are the first person who spotted that nit.
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>>
Hi,
> 44 instances of a leading + in mesa/main compared to 78 trailing ones.
> Huh, I was going to say that it's really uncommon to do this in mesa, but
> that may not be supported by fact.
Considering there is a formatting issue below, I can change it to trailing. I
don’t care.
>> + if (
> FWIW, the kernel standards for commit messages are at:
> https://www.kernel.org/doc/Documentation/SubmittingPatches
> Most of those rules apply to Mesa too. It says the body should be wrapped to
> 75 chars (although I've been using 72 like Matt said).
This is my point: "use most rules, but no
> I suppose it could be useful, but I think we've been mostly successful at
> just expecting people to recognize when what they're writing doesn't look
> like the code around it.
This is my point. Older code had different style/expectations than newer code.
For this patch series, I have hit a
HI,
>Or 78 columns, to be safe, but there's exceptions, like if you're
> defining a big static table/array of info.
Uggg I don't mind exceptions, but knowing them is key.
>> 4. successive parenthesis must have spaces between parenthesis
> Example?
if (some_func(some_argument))
is
> Strange because of the types -- presumably fb_samples is an int because its
> uses are in a comparison with another int (that probably doesn't need to be
> an int :)
I am really hesitant to start changing types all over the place; I have a
sinking suspicion that changing the types of fb_width
> This line is too long. (It will not fit in 80 columns in git log since git
> log adds some spaces before each commit message line.)
What is the accepted maximum length for a line in a commit message?
>> - extension table
>> - additions to gl_framebuffer
>>
>> v1 -> v2
>> Spacing and trail
>> src/mesa/drivers/dri/i965/brw_misc_state.c | 10 +++---
> > src/mesa/drivers/dri/i965/brw_sf_state.c | 8
> As is this?
brw_misc_state.c is active for all Gens. The change to brw_sf_state.c is to
add a comment warning that using gl_framebuffer::Width and so o
> Extra space between . and "
Fix I will
> Extra space before .
Fix I will
> Also, if anyone is ever grepping for MAX_FRAMEBUFFER_SAMPLES, they won't find
> this. I'd move the whole word to the next line.
Ok.
> + *
> + * The same requirements are also in place for GL 4.5,
> +
> I'm happy to see new documentation, so thanks for writing it up!
> But let's separate this from the functional changes related to implementing
> the extension. (Didn't I give this comment last time?)
If you did, I missed it. Unless there are objections, I'll remove this from the
series and m
urally, all drivers now use the field from
Visual. If gl_framebuffer::Samples or potentially a better named
gl_framebuffer::_Samples is added, then should the field be removed from Visual
and all references in drivers to use the new field?
-Kevin
-Original Message-
From: Rogovin, Kevin
Hi,
> I think this check should be put in a utility function up in core Mesa
> somewhere. It's open-coded twice in this patch, and the check will change
> when GL_ARB_image_load_store lands.
Would a check of the form:
inline bool
_mesa_has_atomic_ops(struct gl_context *ctx)
{
return ctx-
One more question:
for patch 2 of the series, there was the request to change the type of
_HasAttachments from GLboolean to bool. If the helper functions "survive",
should the helper functions return unsigned int instead of GLuint?
-Kevin
-Original Message-----
From: Rogovin, K
> You haven't been running 'make check'. :) You also need to update
> src/mesa/tests/dispatch_sanity.cpp.
There is something wrong with my box or something... I did run make check and
there were no failures. Out of paranoia, I also ran
src/mesa/main/tests/main-test explicitly and there were no
> I'm waffling about this a bit. I'm concerned that people will use
> buffer->Width when they should use the accessor. I don't see any
> changes to core Mesa code to use the accessor, so I'm a little concerned that
> some subtle, incorrect behavior is introduced... but there may well not be.
>
HI,
> For both this and get_framebuffer_parameteriv below, I don't see the value in
> splitting the implementations. Also, these functions need to check that the
> extension is enabled and generate GL_INVALID_OPERATION if it is not.
No worries, I can add the GL_INVALID_OPERATION bit in. I ha
Hi,
One comment on the code, or rather a request to the reviewer of the code. The
icky part of checking this patch is correct is checking that the remaining
instances of gl_framebuffer::Width, Height, MaxNumLayers and Visaul.samples are
"correct". I believe I "got 'em all", of those that shoul
I just want to make a begging on the review for this patch: I am a touch
paranoid
about how the thing will act under the GL ES situation; I believe it should
follow
the spec, but if whoever reviews does the extra leg work of checking that I got
this
right, I'd really appreciate it.
_
Hi,
> I'd just go read the ES 3.1 spec and see if there are any differences in this
> area.
I checked the spec, and it appears to me to have the same behavior as
GL_ARB_framebuffer_no_attachments.
> Also, please fix your mail client to stop its weird line wrapping (and the
> other half of the
> When you rebase this, I'd appreciate it if you could insert it into
> the list in alphabetical order. (You based this on a commit where a
> bunch of the later additions were already not inserted alphabetically,
> but I've recently fixed that up.)
Sure, no worries. Given that I am just changing t
> At the bottom is another block with GLES 3.1 requirements, which also
> contains GL_ARB_f_n_a.
At first, I said "Oh futz, I did not mark that one". Then I did some thinking.
Before expressing
my thoughts I want to emphasize that I really do not know what is the best
answer, or
potentially ev
Hello,
> No, because the non-shared code is (by your own admission) untested and/or
> dead code. Untested code is broken code. I would personally be ok with a
> lot > of the changes that just replace fb->Width with
> _mesa_geometric_width(fb) since it's effectively just replacing a direct
>
> I read the patch again and I'm still in the opinion that the changes to the
> pure pre-gen7 logic (i.e., logic that is not re-used for later gens) are not
> needed.
As I have tried and apparently failed to communicate, it is -better- and more
consistent. Need
is a far stronger word. Without
I wrote the patch 3 so that it is
trivial to
implement the DSA function though.
-Kevin
On Fri, Apr 24, 2015 at 11:06 AM, Rogovin, Kevin
wrote:
> Hi,
>
> I agree with the comments about the code (and when the last element of the
> series is reviewed I will submit the series with review co
Hi,
> This is in fact two changes: introduction of the helpers and refactoring of
> the intersection code to be used with caller provided bounding box.
Is this a request to change the commit message or to split this as well?
I think splitting it is silly, but if it make you happy so be it; the
Hi,
>A couple of us chatted about this, and we all agreed that it's probably not
>useful to
>enable ARB_framebuffer_no_attachments on pre-Gen7. We don't expose atomics,
>SSBOs,
> or image load/store on those platforms (and probably won't), so the
> only way fragment shaders can output any d
ngies".
-Kevin
-Original Message-
From: Pohjolainen, Topi
Sent: Monday, April 27, 2015 10:43 AM
To: Rogovin, Kevin
Cc: mesa-...@freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 5/7] i965: use
_mesa_geometry_width/height/layers/samples for programming geometry of
framebuffer to GEN
present in both gen7_atoms
and gen8_atoms.
I would argue that _CurrentFragmentProgram can be NULL, given
that other places check for it and that without the check piglit gets
about 30 more crashes.
Sorry for not posting this in the first reply.
-Kevin
-Original Message-
From: Rogovin
tions when programming the rasterizer/windower
thingies.
-Kevin
-Original Message-----
From: Rogovin, Kevin
Sent: Friday, April 24, 2015 7:43 PM
To: Pohjolainen, Topi
Cc: mesa-...@freedesktop.org
Subject: RE: [Mesa-dev] [PATCH 5/7] i965: use
_mesa_geometry_width/height/layers/samples for p
> Checking brw->ctx.Shader._CurrentFragmentProgram != NULL is unnecessary.
> There is always a valid pixel shader. (If the application is using
> fixed-function, we supply a fragment shader for them.) Please drop that
> check.
Without this check(in the Gen7 function/code), about 30 crashes a
> I'll check with Jordan and others. I have a faint recollection that compute
> shaders have similar
> needs. I think your change is fine though, I just want to understand the
> bigger picture first.
I do not think compute shaders have similar needs. These flags are for making
sure the
rast
1 - 100 of 148 matches
Mail list logo
