[igt-dev] [PATCH i-g-t v5 2/4] lib: Remove duplications in gpu_fill library

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 02:14:35PM +0100, Kalamarz, Lukasz wrote:
> On Tue, 2018-04-10 at 13:34 +0200, Katarzyna Dec wrote:
> > After moving all functions needed for gpgpu and media fill testing
> > there is a lot of duplications which can be removed:
> >   Library media_fill_gen8 and media_fill_gen8lp for CHT was removed,
> > media state flush for !CHT was added to gen7_emit_media_objects.
> >   Many gen8 functions were replaced with gen7 version with devid
> > parameter (gen7_fill_curbe_load, gen7_emit_interface_descriptor,
> > gen7_fill_binding_table, gen7_emit_media_objects). Unified fill
> > kernel
> > function so it is applicable to all gens and both media and gpgpu
> > (merged gen7_fill_media_kernel and gen8_fill_media_kernel).
> >   Duplicated constants like GEN8_MEDIA_VFE_STATE,
> > GEN8_MEDIA_CURBE_LOAD,
> > GEN8_MEDIA_INTERFACE_DESCRIPTOR_LOAD, GEN8_MEDIA_OBJECT were
> > replaced by GEN7 version. However this constants were not removed
> > from gen8_media.h library, because they are used by other tests
> > for Gen8+. More refactoring in this gen*_media.h libraries is needed.
> > 
> > It seems that further unification of *_fillfunc functions will
> > introduce more confusion in understanding what the tests are doing
> > and what were changes between Gens.
> > 
> > v2: Moved some reduntant changes from Move gpgpu/media fill to
> > gpu_fill...
> > to this patch. Applied comments from review.
> > 
> > v3: rebase
> > 
> > Signed-off-by: Katarzyna Dec <katarzyna.dec at intel.com>
> > Cc: Lukasz Kalamarz <lukasz.kalamarz at intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> There are some registers or their values, which are still duplicated
> under different name. I assume that after this series will be merged,
> You will work on refactoring header files. If not, then please fix
> entries mentioned below.
> 
> Reviewed-by: Lukasz Kalamarz <lukasz.kalamarz at intel.com> 
> > ---
> > 
> GEN7_SURFACE_2D == GEN8_SURFACE_2D
> GEN7_SURFACEFORMAT_R8_UNORM == GEN8_SURFACEFORMAT_R8_UNORM
> GEN7_FLOATING_POINT_IEEE_754 == GEN8_FLOATING_POINT_IEEE_754
> GEN7_STATE_BASE_ADDRESS == GEN8_STATE_BASE_ADDRESS
> GEN7_PIPELINE_SELECT == GEN8_PIPELINE_SELECT
>
Hi Lukasz,
I know that this registers are still duplicated. The ones I touched
allowed to remove duplicated functions such as:
gen7_emit_vfe_state_gpgpu(batch);
gen7_emit_curbe_load(batch, curbe_buffer);
gen7_emit_interface_descriptor_load(batch, interface_descriptor);.
Examples you've mentioned are also connected with media_spin test.
Further refactoring in genX_media.h and media_spin lib is planned
after this patch and patch with batch_* will be accepted.

Thanks for the review :)
Kasia