[Mesa-dev] [PATCH V2 00/24] Add Cannonlake support
Anuj Phogat
anuj.phogat at gmail.com
Fri Jun 9 21:44:47 UTC 2017
On Thu, Jun 8, 2017 at 5:23 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> I think I've now reviewed everything except 2 patches. For the "Update a
> few assertions" patch, you said you would run a test but never reported back
> the results. The other is the patch for re-enabling sRGB fast-clears. That
> one isn't needed for enabling and I'm not yet convinced that it's removing
> enough code. I need to understand Sky lake sRGB myself before I can really
> review anything. Also, at Chad's request, I'll probably be adjusting the
> way that code works to be based on the ISL table at which point, re-enabling
> sRGB will just naturally fall out as a result of the ISL format table
> update.
>
Thanks for reviewing rest of the patches. I'll hold back re-enabling sRGB
fast-clears patch. I've also posted new comments on "Update a few assertions"
patch.
> On Mon, Jun 5, 2017 at 10:04 AM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
>>
>> For your reference, here is a list of patches pending review in this
>> series:
>> 3, 18, 19, 22, 23, 24.5.
>>
>> Thanks
>> Anuj
>>
>> On Fri, Jun 2, 2017 at 5:48 PM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
>> > On Fri, Jun 2, 2017 at 4:48 PM, Jason Ekstrand <jason at jlekstrand.net>
>> > wrote:
>> >> On Mon, May 22, 2017 at 9:32 AM, Anuj Phogat <anuj.phogat at gmail.com>
>> >> wrote:
>> >>>
>> >>> On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat <anuj.phogat at gmail.com>
>> >>> wrote:
>> >>> > This series adds support for Cannonlake.
>> >>> >
>> >>> > Changes from V1 to V2:
>> >>> > - Incorporated the review comments from V1.
>> >>> > - Rebased 8 months old CNL branch on top of master
>> >>> > - Wired up Linux and Android build files for gen10
>> >>> > - Replaced the use of few gen9 functions with gen10 specific
>> >>> > functions.
>> >>> > - Squashed few patches, dropped few and created new patches.
>> >>> >
>> >>> Thanks to Jason and Ken who have reviewed few patches in this series.
>> >>> Rest of them are still waiting for the review. I really want to land
>> >>> this
>> >>> series
>> >>> (at least first 15-16 patches) soon. There are some very easy patches
>> >>> any
>> >>> one can review like enabling Mesa to build for gen10 etc. Please take
>> >>> a
>> >>> look at them. Thanks :).
>> >>
>> >>
>> >> Finally got around to looking at these again...
>> >>
>> >> Now that we're switching everything over to genxml, I think it's a good
>> >> idea
>> >> to be a bit more intentional in the way we write new platform patches.
>> >> There are a number of patches in this series that are much harder to
>> >> review
>> >> than they need to be because they're written more-or-less in order of
>> >> code
>> >> development and not in a logical reviewable order. In particular,
>> >> there's a
>> >> patch which updates a pile of switch statements to get rid of asserts
>> >> but it
>> >> just moves them all over to gen9. Moving stuff to gen10 is in a
>> >> different
>> >> patch. The result is that it's very hard, without squashing things
>> >> together, to tell whether or not we missed anything when we switched
>> >> them
>> >> over to actual gen10 functions. This isn't really a criticism of Anuj
>> >> and
>> >> Ben. They've done a lot of rebasaing on top of a lot of driver
>> >> architecture
>> >> changes. I think this will be much easier to do better in the future.
>> >>
>> >> In my view, the ideal platform enabling patch series would look
>> >> something
>> >> like this:
>> >>
>> >> 1) Add genN.xml
>> > Does it make sense to break it down in to few patches based on manual
>> > changes we make to an auto generated genN.xml ? or send out just one
>> > patch and reviewer can diff it with previous gen and verify the changes
>> > ?
>> >
>> >> 2) Add the #defines and #includes to genxml and the build system stuff
>> >> to
>> >> generate the packing headers
>> >> 3) Update stuff in src/intel/common such as URB configuration changes
>> >> 4) Update ISL:
>> >> a) Any needed generic ISL changes such as adding new layaouts.
>> >> (Cannon
>> >> lake doesn't add anything, so nothing to do here). This may be
>> >> multiple
>> >> patches.
>> >> b) Get ISL surface state emit code building for the new hardware.
>> >> This
>> >> includes updating the autotools and Android makefiles, adding function
>> >> prototypes, updating switch statements, etc. If changes are needed in
>> >> isl_surface_state.c or isl_depth_stencil.c, they should be minimal bug
>> >> still
>> >> enough that the end result is correct.
>> >> 5) Update BLORP as needed for the new platform. Sadly, there's no way
>> >> to
>> >> build-test this without the next step since BLORP doesn't build its own
>> >> genX
>> >> files.
>> >> 6) Get GL driver genxml state-upload and blorp code building and hooked
>> >> in.
>> >> Core blorp changes should go in their own patch (above) but this will
>> >> include the build system changes for blorp as well. This also includes
>> >> updating switch statements.
>> >> 8) Implement workarounds, features, etc.
>> >>
>> >> The important part is that we keep related changes together. In order
>> >> to
>> >> review this series, I had to squash patches 6, 9, 14, and 15 together.
>> >> Again, I'm not casting any blame here. The series makes a lot of
>> >> historical
>> >> sense. It's just hard to review as-is.
>> >>
>> > I agree with you that the series could have been structured better for
>> > ease
>> > of review. Thanks for the reviews and all the great suggestions above.
>> > I'll
>> > keep them in mind for future h/w enabling patches.
>> >
>> >> --Jason
>> >>
>> >>>
>> >>> > What's remaining:
>> >>> > - Add missing gen10 bits in Vulkan driver.
>> >>> > - Fix failing piglit, cts tests for GL and Vulkan.
>> >>> >
>> >>> > You can also find this series at:
>> >>> > https://github.com/aphogat/mesa.git
>> >>> > branch: reviews
>> >>> >
>> >>> > Anuj Phogat (18):
>> >>> > i965/cnl: Define genX(x) and GENX(x) for gen10
>> >>> > i965/cnl: Include gen10_pack.h
>> >>>
>> >>> > i965/cnl: Add gen10 specific function declarations
>> >>> > i965/cnl: Update the script generating genX_bits.h
>> >>> > i965/cnl: Add isl_gen10 header and source files
>> >>> > i965/cnl: Wire up Mesa build files for gen10
>> >>> > i965/cnl: Wire up android Mesa build files for gen10
>> >>> > i965/cnl: Add pci id for INTEL_DEVID_OVERRIDE
>> >>> > i965/cnl: Add cnl bits in aubinator
>> >>> > i965/cnl: Update few assertions
>> >>> > i965/cnl: Handle gen10 in switch cases across the driver
>> >>> > i965/cnl: Start using CNL MOCS defines
>> >>> > i965/cnl: Start using gen10 specific functions
>> >>> > i965/cnl: Don't resolve single sampled color rb in case of sRGB
>> >>> > formats
>> >>> > i965/cnl: Make URB {VS, GS, HS, DS} sizes non multiple of 3
>> >>> > i965/cnl: Reformat surface_format_info table to accomodate gen10+
>> >>> > i965/cnl: Enable CCS_E and RT support for few formats
>> >>> > i965: Simplify get_l3_way_size() function
>> >>> >
>> >>> > Ben Widawsky (5):
>> >>> > i965: Make feature macros gen8 based
>> >>> > i965/cnl: Add a preliminary device for Cannonlake
>> >>> > i965/cnl: Implement new pipe control workaround
>> >>> > i965/cnl: Implement depth count workaround
>> >>> > i965/cnl: Restore lossless compression for sRGB formats
>> >>> >
>> >>> > Jason Ekstrand (1):
>> >>> > i965/cnl: Add gen10.xml
>> >>> >
>> >>> > include/pci_ids/i965_pci_ids.h | 12 +
>> >>> > src/intel/Android.genxml.mk | 5 +
>> >>> > src/intel/Android.isl.mk | 20 +
>> >>> > src/intel/Android.vulkan.mk | 21 +
>> >>> > src/intel/Makefile.isl.am | 4 +
>> >>> > src/intel/Makefile.sources | 12 +-
>> >>> > src/intel/Makefile.vulkan.am | 7 +-
>> >>> > src/intel/common/gen_device_info.c | 71 +-
>> >>> > src/intel/common/gen_device_info.h | 1 +
>> >>> > src/intel/common/gen_l3_config.c | 11 +-
>> >>> > src/intel/compiler/brw_compiler.h | 2 +-
>> >>> > src/intel/compiler/brw_eu.c | 2 +
>> >>> > src/intel/compiler/brw_eu_compact.c | 1 +
>> >>> > src/intel/genxml/gen10.xml | 3563
>> >>> > ++++++++++++++++++++++
>> >>> > src/intel/genxml/genX_pack.h | 2 +
>> >>> > src/intel/genxml/gen_bits_header.py | 6 +-
>> >>> > src/intel/genxml/gen_macros.h | 3 +
>> >>> > src/intel/isl/isl.c | 9 +
>> >>> > src/intel/isl/isl_format.c | 498 +--
>> >>> > src/intel/isl/isl_gen10.c | 41 +
>> >>> > src/intel/isl/isl_gen10.h | 45 +
>> >>> > src/intel/isl/isl_priv.h | 12 +
>> >>> > src/intel/tools/aubinator.c | 8 +-
>> >>> > src/intel/vulkan/anv_cmd_buffer.c | 1 +
>> >>> > src/intel/vulkan/anv_device.c | 1 +
>> >>> > src/intel/vulkan/anv_entrypoints_gen.py | 1 +
>> >>> > src/mesa/drivers/dri/i965/Android.mk | 24 +-
>> >>> > src/mesa/drivers/dri/i965/Makefile.am | 6 +-
>> >>> > src/mesa/drivers/dri/i965/Makefile.sources | 4 +
>> >>> > src/mesa/drivers/dri/i965/brw_blorp.c | 6 +
>> >>> > src/mesa/drivers/dri/i965/brw_blorp.h | 2 +
>> >>> > src/mesa/drivers/dri/i965/brw_context.c | 2 +-
>> >>> > src/mesa/drivers/dri/i965/brw_formatquery.c | 1 +
>> >>> > src/mesa/drivers/dri/i965/brw_pipe_control.c | 11 +
>> >>> > src/mesa/drivers/dri/i965/brw_program.c | 2 +-
>> >>> > src/mesa/drivers/dri/i965/brw_queryobj.c | 8 +
>> >>> > src/mesa/drivers/dri/i965/brw_state.h | 9 +
>> >>> > src/mesa/drivers/dri/i965/brw_state_upload.c | 4 +-
>> >>> > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 +
>> >>> > src/mesa/drivers/dri/i965/gen7_urb.c | 12 +
>> >>> > src/mesa/drivers/dri/i965/genX_state_upload.c | 4 +-
>> >>> > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 2 +-
>> >>> > src/mesa/drivers/dri/i965/intel_screen.c | 2 +
>> >>> > 43 files changed, 4180 insertions(+), 280 deletions(-)
>> >>> > create mode 100644 src/intel/genxml/gen10.xml
>> >>> > create mode 100644 src/intel/isl/isl_gen10.c
>> >>> > create mode 100644 src/intel/isl/isl_gen10.h
>> >>> >
>> >>> > --
>> >>> > 2.9.3
>> >>> >
>> >>> _______________________________________________
>> >>> mesa-dev mailing list
>> >>> mesa-dev at lists.freedesktop.org
>> >>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> >>
>> >>
>
>
More information about the mesa-dev
mailing list