[Mesa-dev] [PATCH V2 00/24] Add Cannonlake support

Anuj Phogat anuj.phogat at gmail.com
Mon Jun 5 17:04:17 UTC 2017


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