Re: [Mesa-dev] [Review Request (master branch)] svga: Use pipe_shader_state_from_tgsi to set shader state
Those two patches are simple fixes to our svga driver and have been tested in house for a while already. So I thought it's ok to push the patches directly. Thanks for reminding me of the MR practice, will keep that in mind for next submits. -Charmaine From: Michel Dänzer Sent: Tuesday, February 11, 2020 2:03 AM To: Neha Bhende; Brian Paul; Charmaine Lee Cc: mesa-dev@lists.freedesktop.org Subject: Re: [Mesa-dev] [Review Request (master branch)] svga: Use pipe_shader_state_from_tgsi to set shader state Hi Charmaine, it looks like you pushed this patch and another one directly to the main Mesa repository master branch. Pushing directly to the main Mesa repository is no longer common practice, and indeed discouraged, as it circumvents the pre-merge GitLab CI pipeline and forfeits all other benefits of a merge request (MR). The common practice is to create an MR (normally already for patch review) and assign it to Marge Bot for merging. Marge will rebase as needed and merge once the pre-merge CI pipeline has passed. Thanks, On 2020-01-29 5:14 p.m., Neha Bhende wrote: > Use pipe_shader_state_from_tgsi() to set shader state for transformed > shader so that we get all correct data for respective shader state. > > This fixes several regressed glretrace, piglit crashes found during merging > upsteam mesa > > Fixes: bf12bc2dd7a2 (draw: add nir info gathering and building support) > > Reviewed-by: Charmaine Lee > --- > src/gallium/drivers/svga/svga_state_tgsi_transform.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/svga/svga_state_tgsi_transform.c > b/src/gallium/drivers/svga/svga_state_tgsi_transform.c > index b567aab6bc8..9d701b73772 100644 > --- a/src/gallium/drivers/svga/svga_state_tgsi_transform.c > +++ b/src/gallium/drivers/svga/svga_state_tgsi_transform.c > @@ -131,7 +131,7 @@ emulate_point_sprite(struct svga_context *svga, > tgsi_dump(new_tokens, 0); >} > > - templ.tokens = new_tokens; > + pipe_shader_state_from_tgsi(&templ, new_tokens); >templ.stream_output.num_outputs = 0; > >if (streamout) { > -- Earthling Michel Dänzer | https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fredhat.com&data=02%7C01%7Ccharmainel%40vmware.com%7Cc04e2bb7e84a46b9879008d7aed996f5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637170121886003685&sdata=J3VtiMG%2Ba5Co3m3nrjVULvj6a9QcpyIYkzpQrbx%2BR70%3D&reserved=0 Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Review Request (master branch)] svga: Use pipe_shader_state_from_tgsi to set shader state
On 2020-02-11 6:49 p.m., Charmaine Lee wrote: > > Those two patches are simple fixes to our svga driver and have been tested in > house for a while already. > So I thought it's ok to push the patches directly. > Thanks for reminding me of the MR practice, will keep that in mind for next > submits. Thanks. The CI pipeline does test building the svga driver (maybe the testing coverage can be increased in the future? :), so it's at least theoretically possible for an svga driver change to break the pipeline. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Review Request (master branch)] svga: Use pipe_shader_state_from_tgsi to set shader state
I'm going to update the docs regarding patches and gitlab. It's kind of a mess now. -Brian On 02/11/2020 03:03 AM, Michel Dänzer wrote: Hi Charmaine, it looks like you pushed this patch and another one directly to the main Mesa repository master branch. Pushing directly to the main Mesa repository is no longer common practice, and indeed discouraged, as it circumvents the pre-merge GitLab CI pipeline and forfeits all other benefits of a merge request (MR). The common practice is to create an MR (normally already for patch review) and assign it to Marge Bot for merging. Marge will rebase as needed and merge once the pre-merge CI pipeline has passed. Thanks, On 2020-01-29 5:14 p.m., Neha Bhende wrote: Use pipe_shader_state_from_tgsi() to set shader state for transformed shader so that we get all correct data for respective shader state. This fixes several regressed glretrace, piglit crashes found during merging upsteam mesa Fixes: bf12bc2dd7a2 (draw: add nir info gathering and building support) Reviewed-by: Charmaine Lee --- src/gallium/drivers/svga/svga_state_tgsi_transform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/drivers/svga/svga_state_tgsi_transform.c b/src/gallium/drivers/svga/svga_state_tgsi_transform.c index b567aab6bc8..9d701b73772 100644 --- a/src/gallium/drivers/svga/svga_state_tgsi_transform.c +++ b/src/gallium/drivers/svga/svga_state_tgsi_transform.c @@ -131,7 +131,7 @@ emulate_point_sprite(struct svga_context *svga, tgsi_dump(new_tokens, 0); } - templ.tokens = new_tokens; + pipe_shader_state_from_tgsi(&templ, new_tokens); templ.stream_output.num_outputs = 0; if (streamout) { ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Review Request (master branch)] svga: Use pipe_shader_state_from_tgsi to set shader state
Hi Charmaine, it looks like you pushed this patch and another one directly to the main Mesa repository master branch. Pushing directly to the main Mesa repository is no longer common practice, and indeed discouraged, as it circumvents the pre-merge GitLab CI pipeline and forfeits all other benefits of a merge request (MR). The common practice is to create an MR (normally already for patch review) and assign it to Marge Bot for merging. Marge will rebase as needed and merge once the pre-merge CI pipeline has passed. Thanks, On 2020-01-29 5:14 p.m., Neha Bhende wrote: > Use pipe_shader_state_from_tgsi() to set shader state for transformed > shader so that we get all correct data for respective shader state. > > This fixes several regressed glretrace, piglit crashes found during merging > upsteam mesa > > Fixes: bf12bc2dd7a2 (draw: add nir info gathering and building support) > > Reviewed-by: Charmaine Lee > --- > src/gallium/drivers/svga/svga_state_tgsi_transform.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/svga/svga_state_tgsi_transform.c > b/src/gallium/drivers/svga/svga_state_tgsi_transform.c > index b567aab6bc8..9d701b73772 100644 > --- a/src/gallium/drivers/svga/svga_state_tgsi_transform.c > +++ b/src/gallium/drivers/svga/svga_state_tgsi_transform.c > @@ -131,7 +131,7 @@ emulate_point_sprite(struct svga_context *svga, > tgsi_dump(new_tokens, 0); >} > > - templ.tokens = new_tokens; > + pipe_shader_state_from_tgsi(&templ, new_tokens); >templ.stream_output.num_outputs = 0; > >if (streamout) { > -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev