On Fri, Aug 5, 2016 at 1:23 PM, Anuj Phogat <anuj.pho...@gmail.com> wrote:
>
> On Fri, Aug 5, 2016 at 10:05 AM, Lionel Landwerlin <
> lionel.g.landwer...@intel.com> wrote:
>
>> Fixes 6 failures from dEQP-VK.api.copy_and_blit.resolve_image.*
>>
> No tests are run with --deqp-case=dEQP-VK.api.copy_a
> nd_blit.resolve_image.*
>
> This is what i'm seeing on hsw without this patch:
> ./deqp-vk --deqp-case=dEQP-VK.api.copy_and_blit*
> Test run totals:
> Passed: 9/11 (81.8%)
> Failed: 0/11 (0.0%)
> Not supported: 2/11 (18.2%)
> Warnings: 0/11 (0.0%)
>
>>
>> Tested on IVB/HSW
>>
>> v2: Program multisample dispatch mode correctly
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
>> ---
>> src/intel/vulkan/gen7_pipeline.c | 46 ++++++++++++++++++++++++++----
>> ----------
>> 1 file changed, 30 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/intel/vulkan/gen7_pipeline.c
>> b/src/intel/vulkan/gen7_pipeline.c
>> index 6acdd85..2611a2d 100644
>> --- a/src/intel/vulkan/gen7_pipeline.c
>> +++ b/src/intel/vulkan/gen7_pipeline.c
>> @@ -36,27 +36,34 @@
>>
>> static void
>> gen7_emit_rs_state(struct anv_pipeline *pipeline,
>> - const VkPipelineRasterizationStateCreateInfo *info,
>> + const VkPipelineRasterizationStateCreateInfo
>> *rs_info,
>> + const VkPipelineMultisampleStateCreateInfo *ms_info,
>> const struct anv_graphics_pipeline_create_info
>> *extra)
>> {
>> + uint32_t samples = 1;
>> +
>> + if (ms_info)
>> + samples = ms_info->rasterizationSamples;
>> +
>> struct GENX(3DSTATE_SF) sf = {
>> GENX(3DSTATE_SF_header),
>>
>> /* LegacyGlobalDepthBiasEnable */
>>
>> .StatisticsEnable = true,
>> - .FrontFaceFillMode =
>> vk_to_gen_fillmode[info->polygonMode],
>> - .BackFaceFillMode =
>> vk_to_gen_fillmode[info->polygonMode],
>> + .FrontFaceFillMode =
>> vk_to_gen_fillmode[rs_info->polygonMode],
>> + .BackFaceFillMode =
>> vk_to_gen_fillmode[rs_info->polygonMode],
>> .ViewTransformEnable = !(extra &&
>> extra->use_rectlist),
>> - .FrontWinding =
>> vk_to_gen_front_face[info->frontFace],
>> + .FrontWinding =
>> vk_to_gen_front_face[rs_info->frontFace],
>> /* bool
>> AntiAliasingEnable; */
>>
>> - .CullMode =
>> vk_to_gen_cullmode[info->cullMode],
>> + .CullMode =
>> vk_to_gen_cullmode[rs_info->cullMode],
>>
>> /* uint32_t
>> LineEndCapAntialiasingRegionWidth; */
>> .ScissorRectangleEnable = !(extra &&
>> extra->use_rectlist),
>>
>> - /* uint32_t
>> MultisampleRasterizationMode; */
>> + .MultisampleRasterizationMode = samples > 1 ?
>> + MSDISPMODE_PERPIXEL : MSDISPMODE_PERSAMPLE,
>> /* bool LastPixelEnable; */
>>
>> .TriangleStripListProvokingVertexSelect = 0,
>> @@ -67,9 +74,9 @@ gen7_emit_rs_state(struct anv_pipeline *pipeline,
>> /* uint32_t
>> VertexSubPixelPrecisionSelect; */
>> .UsePointWidthState = false,
>> .PointWidth = 1.0,
>> - .GlobalDepthOffsetEnableSolid = info->depthBiasEnable,
>> - .GlobalDepthOffsetEnableWireframe = info->depthBiasEnable,
>> - .GlobalDepthOffsetEnablePoint = info->depthBiasEnable,
>> + .GlobalDepthOffsetEnableSolid =
>> rs_info->depthBiasEnable,
>> + .GlobalDepthOffsetEnableWireframe =
>> rs_info->depthBiasEnable,
>> + .GlobalDepthOffsetEnablePoint =
>> rs_info->depthBiasEnable,
>> };
>>
>> GENX(3DSTATE_SF_pack)(NULL, &pipeline->gen7.sf, &sf);
>> @@ -108,7 +115,8 @@ genX(graphics_pipeline_create)(
>> emit_vertex_input(pipeline, pCreateInfo->pVertexInputState, extra);
>>
>> assert(pCreateInfo->pRasterizationState);
>> - gen7_emit_rs_state(pipeline, pCreateInfo->pRasterizationState,
>> extra);
>> + gen7_emit_rs_state(pipeline, pCreateInfo->pRasterizationState,
>> + pCreateInfo->pMultisampleState, extra);
>>
> It'll be nice to split the patch in to two:
> - First one carries changes for info -> rs_info.
> - Second one with rest of the changes.
>
On second look we don't need ms_info in gen7_emit_rs_state. We should
use wm_prog_data to set
MultisampleRasterizationMode
. However I noted
that we're not setting this bit on i965.
FYI: I've few wip patches in my 'dev' branch at
https://github.com/aphogat/mesa.git
One of them is duplicating some work in this patch. I'm working on fixing 8
failing
multisample tests (./deqp-vk --deqp-case=dEQP-VK.pipeline.multisample*) with
this branch, before I send out the series for review. That said, I'll be
happy to
land your patch with all comments fixed.
Thanks
Anuj
>
>> emit_ds_state(pipeline, pCreateInfo->pDepthStencilState, pass,
>> subpass);
>>
>> @@ -121,14 +129,13 @@ genX(graphics_pipeline_create)(
>> pCreateInfo->pRasterizationState, extra);
>> emit_3dstate_streamout(pipeline, pCreateInfo->pRasterizationState);
>>
>> - if (pCreateInfo->pMultisampleState &&
>> - pCreateInfo->pMultisampleState->rasterizationSamples > 1)
>> - anv_finishme("VK_STRUCTURE_TYPE_PIPELINE_MULTISAMPLE_STATE_C
>> REATE_INFO");
>> -
>> - uint32_t samples = 1;
>> + uint32_t samples = pCreateInfo->pMultisampleState
>> ->rasterizationSamples;
>> uint32_t log2_samples = __builtin_ffs(samples) - 1;
>>
>> anv_batch_emit(&pipeline->batch, GENX(3DSTATE_MULTISAMPLE), ms) {
>> +#if GEN_IS_HASWELL
>> + ms.MultiSampleEnable = samples > 1;
>> +#endif
>>
> We're not setting this bit in i965 driver without any issues. There is
> also a h/w
> issue listed in docs
> for this bit on hsw.
>
>
>
>> ms.PixelLocation = PIXLOC_CENTER;
>> ms.NumberofMultisamples = log2_samples;
>> }
>> @@ -139,7 +146,7 @@ genX(graphics_pipeline_create)(
>>
>> const struct brw_vs_prog_data *vs_prog_data =
>> get_vs_prog_data(pipeline);
>>
>> -#if 0
>> +#if 0
>> /* From gen7_vs_state.c */
>>
>> /**
>> @@ -233,6 +240,8 @@ genX(graphics_pipeline_create)(
>> wm.LineAntialiasingRegionWidth = 1; /* 1.0 pixels */
>> wm.EarlyDepthStencilControl = EDSC_NORMAL;
>> wm.PointRasterizationRule = RASTRULE_UPPER_RIGHT;
>> + wm.MultisampleDispatchMode = samples > 1 ?
>> + MSDISPMODE_PERPIXEL : MSDISPMODE_PERSAMPLE;
>>
> Use
> wm_prog_data->persample_dispatch
> here. I'm not sure if we really
> need this when there is no fragment shader.
>
>> }
>>
>> /* Even if no fragments are ever dispatched, the hardware hangs if
>> we
>> @@ -312,6 +321,11 @@ genX(graphics_pipeline_create)(
>> }
>>
>> wm.BarycentricInterpolationMode =
>> wm_prog_data->barycentric_interp_modes;
>> +
>> + wm.MultisampleDispatchMode = samples > 1 ?
>> + MSRASTMODE_ON_PATTERN : MSRASTMODE_OFF_PIXEL;
>>
> Use wm_prog_data->persample_dispatch here.
>
>> + wm.MultisampleDispatchMode =
>> wm_prog_data->persample_dispatch ?
>> + MSDISPMODE_PERSAMPLE : MSDISPMODE_PERPIXEL;
>>
> You're setting the same field twice with different values here. Keep the
> 2nd line
> aligned to
>
> "wm_prog_data->persample_dispatch".
>
>> }
>> }
>>
>> --
>> 2.8.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
