On Fri, Apr 24, 2015 at 9:43 AM, Zhou, Jammy <Jammy.Zhou at amd.com> wrote: > Hi Alex, > > For the core driver patch: > > +config DRM_AMDGPU > + tristate "AMD GPU" > + depends on DRM && PCI > + select FB_CFB_FILLRECT > + select FB_CFB_COPYAREA > + select FB_CFB_IMAGEBLIT > + select FW_LOADER > + select DRM_KMS_HELPER > + select DRM_KMS_FB_HELPER > + select DRM_TTM > + select POWER_SUPPLY > + select HWMON > + select BACKLIGHT_CLASS_DEVICE > + select DRM_AMD_GNB_BUS > + select INTERVAL_TREE > > I think DRM_AMD_GNB_BUS is not used, we can probably remove it now. > > +/* TODO: Here are things that needs to be done : > + * - surface allocator & initializer : (bit like scratch reg) should > + * initialize HDP_ stuff on RS600, R600, R700 hw, well anythings > + * related to surface > + * - WB : write back stuff (do it bit like scratch reg things) > + * - Vblank : look at Jesse's rework and what we should do > + * - r600/r700: gart & cp > + * - cs : clean cs ioctl use bitmap & things like that. > + * - power management stuff > + * - Barrier in gart code > + * - Unmappabled vram ? > + * - TESTING, TESTING, TESTING > + */ > + > +/* Initialization path: > + * We expect that acceleration initialization might fail for various > + * reasons even thought we work hard to make it works on most > + * configurations. In order to still have a working userspace in such > + * situation the init path must succeed up to the memory controller > + * initialization point. Failure before this point are considered as > + * fatal error. Here is the init callchain : > + * amdgpu_device_init perform common structure, mutex initialization > + * asic_init setup the GPU memory layout and perform all > + * one time initialization (failure in this > + * function are considered fatal) > + * asic_startup setup the GPU acceleration, in order to > + * follow guideline the first thing this > + * function should do is setting the GPU > + * memory controller (only MC setup failure > + * are considered as fatal) > + */ > + > These should be outdated, and I think they can be removed now. > > For the uapi header patch: > > +#define AMDGPU_TILING_MACRO 0x1 > +#define AMDGPU_TILING_MICRO 0x2 > +#define AMDGPU_TILING_SWAP_16BIT 0x4 > +#define AMDGPU_TILING_R600_NO_SCANOUT > AMDGPU_TILING_SWAP_16BIT > +#define AMDGPU_TILING_SWAP_32BIT 0x8 > +/* this object requires a surface when mapped - i.e. front buffer */ > +#define AMDGPU_TILING_SURFACE 0x10 > +#define AMDGPU_TILING_MICRO_SQUARE 0x20 > +#define AMDGPU_TILING_EG_BANKW_SHIFT 8 > +#define AMDGPU_TILING_EG_BANKW_MASK 0xf > +#define AMDGPU_TILING_EG_BANKH_SHIFT 12 > +#define AMDGPU_TILING_EG_BANKH_MASK 0xf > +#define AMDGPU_TILING_EG_MACRO_TILE_ASPECT_SHIFT 16 > +#define AMDGPU_TILING_EG_MACRO_TILE_ASPECT_MASK 0xf > +#define AMDGPU_TILING_EG_TILE_SPLIT_SHIFT 24 > +#define AMDGPU_TILING_EG_TILE_SPLIT_MASK 0xf > +#define AMDGPU_TILING_EG_STENCIL_TILE_SPLIT_SHIFT 28 > +#define AMDGPU_TILING_EG_STENCIL_TILE_SPLIT_MASK 0xf > ...... > +#define SI_TILE_MODE_COLOR_LINEAR_ALIGNED 8 > +#define SI_TILE_MODE_COLOR_1D 13 > +#define SI_TILE_MODE_COLOR_1D_SCANOUT 9 > +#define SI_TILE_MODE_COLOR_2D_8BPP 14 > +#define SI_TILE_MODE_COLOR_2D_16BPP 15 > +#define SI_TILE_MODE_COLOR_2D_32BPP 16 > +#define SI_TILE_MODE_COLOR_2D_64BPP 17 > +#define SI_TILE_MODE_COLOR_2D_SCANOUT_16BPP 11 > +#define SI_TILE_MODE_COLOR_2D_SCANOUT_32BPP 12 > +#define SI_TILE_MODE_DEPTH_STENCIL_1D 4 > +#define SI_TILE_MODE_DEPTH_STENCIL_2D 0 > +#define SI_TILE_MODE_DEPTH_STENCIL_2D_2AA 3 > +#define SI_TILE_MODE_DEPTH_STENCIL_2D_4AA 3 > +#define SI_TILE_MODE_DEPTH_STENCIL_2D_8AA 2 > + > +#define CIK_TILE_MODE_DEPTH_STENCIL_1D 5 > > It looks these definitions are not used by libdrm_amdgpu anymore (and even by > the kernel driver). Maybe we can remove the unused definitions, and move the > used ones to amdgpu.h instead. Besides, I think we'd better remove > 'R600/EG/SI/CIK' from the naming.
The AMDGPU_TILING definitions are used by UMDs to set amdgpu_bo_metadata::tiling_info. I plan to rework them, because they are not sufficient for VI and contain obsolete stuff. The SI/CIK_TILE_MODE definitions can be removed indeed. Marek
