[Mesa-dev] [PATCH V2 00/24] Add Cannonlake support
Anuj Phogat
anuj.phogat at gmail.com
Sat Jun 3 00:48:35 UTC 2017
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