On 2022-02-07 at 15:37:09 +0100, Christian König wrote: > Am 07.02.22 um 14:53 schrieb Ramalingam C: > > On 2022-02-07 at 12:41:59 +0100, Christian König wrote: > > > Am 07.02.22 um 10:37 schrieb Ramalingam C: > > > > On flat-ccs capable platform we need to evict and resore the ccs data > > > > along with the corresponding main memory. > > > > > > > > This ccs data can only be access through BLT engine through a special > > > > cmd ( ) > > > > > > > > To support above requirement of flat-ccs enabled i915 platforms this > > > > series adds new param called ccs_pages_needed to the ttm_tt_init(), > > > > to increase the ttm_tt->num_pages of system memory when the obj has the > > > > lmem placement possibility. > > > Well question is why isn't the buffer object allocated with the extra space > > > in the first place? > > Hi Christian, > > > > On Xe-HP and later devices, we use dedicated compression control state (CCS) > > stored in local memory for each surface, to support the 3D and media > > compression formats. > > > > The memory required for the CCS of the entire local memory is 1/256 of the > > local memory size. So before the kernel boot, the required memory is reserved > > for the CCS data and a secure register will be programmed with the CCS base > > address > > > > So when we allocate a object in local memory we dont need to explicitly > > allocate the space for ccs data. But when we evict the obj into the smem > > to hold the compression related data along with the obj we need smem > > space of obj_size + (obj_size/256). > > > > Hence when we create smem for an obj with lmem placement possibility we > > create with the extra space. > > Exactly that's what I've been missing in the cover letter and/or commit > messages, comments etc.. > > Over all sounds like a valid explanation to me, just one comment on the > code/naming: > > > int ttm_tt_init(struct ttm_tt *ttm, struct ttm_buffer_object *bo, > > - uint32_t page_flags, enum ttm_caching caching) > > + uint32_t page_flags, enum ttm_caching caching, > > + unsigned long ccs_pages) > > Please don't try to leak any i915 specific stuff into common TTM code. > > For example use the wording extra_pages instead of ccs_pages here. > > Apart from that looks good to me, Thank you. I will address the comments on naming. Ram > Christian. > > > > > Ram. > > > Regards, > > > Christian. > > > > > > > This will be on top of the flat-ccs enabling series > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F95686%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7Ce54bb7576a334a76cab008d9ea4138e5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637798388115252727%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=V9wQZvb0JwtplIBSYYXGzrg%2BEMvn4hfkscziPFDvZDY%3D&reserved=0 > > > > > > > > For more about flat-ccs feature please have a look at > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F471777%2F%3Fseries%3D95686%26rev%3D5&data=04%7C01%7Cchristian.koenig%40amd.com%7Ce54bb7576a334a76cab008d9ea4138e5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637798388115252727%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=aYjoTKMbZvi%2Fnr7hkSH4SYGxZIv8Dj210dNrBnUNpQw%3D&reserved=0 > > > > > > > > Testing of the series is WIP and looking forward for the early review on > > > > the amendment to ttm_tt_init and the approach. > > > > > > > > Ramalingam C (2): > > > > drm/i915/ttm: Add extra pages for handling ccs data > > > > drm/i915/migrate: Evict and restore the ccs data > > > > > > > > drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- > > > > drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 23 +- > > > > drivers/gpu/drm/i915/gt/intel_migrate.c | 283 +++++++++++---------- > > > > drivers/gpu/drm/qxl/qxl_ttm.c | 2 +- > > > > drivers/gpu/drm/ttm/ttm_agp_backend.c | 2 +- > > > > drivers/gpu/drm/ttm/ttm_tt.c | 12 +- > > > > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 +- > > > > include/drm/ttm/ttm_tt.h | 4 +- > > > > 8 files changed, 191 insertions(+), 139 deletions(-) > > > > >
