[Mesa-dev] [PATCH] radeonsi: Program RASTER_CONFIG for harvested GPUs v4

[Tom Stellard]

On Wed, Sep 17, 2014 at 04:43:12PM +0900, Michel Dänzer wrote:
> On 12.09.2014 03:39, Tom Stellard wrote:
> >On Thu, Sep 11, 2014 at 05:24:03PM +0900, Michel Dänzer wrote:
> >>On 10.09.2014 22:59, Tom Stellard wrote:
> >>>
> >>>+		/* Always use the default config when all backends are enabled. */
> >>>+		if (rb_mask && util_bitcount(rb_mask) < max_backends) {
> >>>+			/* XXX: I can't figure out what the *_XSEL and *_YSEL
> >>>+			 * fields are for, so I'm leaving them as their default
> >>>+			 * values. */
> >>>+			unsigned pkr_mask = (sh_per_se | 0x1);
> >>>+			unsigned se0_pkr0 = rb_mask & pkr_mask;
> >>>+			unsigned se0_pkr1 = (rb_mask >>= sh_per_se) & pkr_mask;
> >>>+			unsigned se1_pkr0 = (rb_mask >>= sh_per_se) & pkr_mask;
> >>>+			unsigned se1_pkr1 = (rb_mask >>= sh_per_se) & pkr_mask;
> >>>+			unsigned se_map = 0;
> >>>+			unsigned se0_pkr_map = 0;
> >>>+			unsigned se1_pkr_map = 0;
> >>>+			unsigned se0_pkr0_rb_map = 0;
> >>>+			unsigned se0_pkr1_rb_map = 0;
> >>>+			unsigned se1_pkr0_rb_map = 0;
> >>>+			unsigned se1_pkr1_rb_map = 0;
> >>>+			if (!se0_pkr0 && !se0_pkr1) {
> >>>+				/* se0 disabled */
> >>>+				se_map |= 0x1;
> >>>+			}
> >>>+			if (se1_pkr0 || se1_pkr1) {
> >>>+				/* se1 enabled */
> >>>+				se_map |= 0x2;
> >>>+			}
> >>>+			if (!se0_pkr0) {
> >>>+				/* se0 pkr0 disabled */
> >>>+				se0_pkr_map |= 0x1;
> >>>+			}
> >>>+			if (se0_pkr1) {
> >>>+				/* se0 pkr1 enabled */
> >>>+				se0_pkr_map |= 0x2;
> >>>+			}
> >>>+			if (!se1_pkr0) {
> >>>+				/* se1 pkr0 disabled */
> >>>+				se1_pkr_map |= 0x1;
> >>>+			}
> >>>+			if (se1_pkr1) {
> >>>+				/* se1 pkr1 enabled */
> >>>+				se1_pkr_map |= 0x2;
> >>>+			}
> >>>+
> >>>+			se0_pkr0_rb_map = pkr_mask_to_map(se0_pkr0);
> >>>+			se0_pkr1_rb_map = pkr_mask_to_map(se0_pkr1);
> >>>+			se1_pkr0_rb_map = pkr_mask_to_map(se1_pkr0);
> >>>+			se1_pkr1_rb_map = pkr_mask_to_map(se1_pkr1);
> >>>+
> >>>+			assert(!se0_pkr0 || !se1_pkr0 || (se0_pkr0_rb_map == se1_pkr0_rb_map));
> >>>+			assert(!se0_pkr1 || !se1_pkr1 || (se0_pkr1_rb_map == se1_pkr1_rb_map));
> >>>+			raster_config &= C_028350_RB_MAP_PKR0;
> >>>+			raster_config |= S_028350_RB_MAP_PKR0(se0_pkr0_rb_map);
> >>>+			raster_config &= C_028350_RB_MAP_PKR1;
> >>>+			raster_config |= S_028350_RB_MAP_PKR1(se0_pkr1_rb_map);
> >>>+			raster_config &= C_028350_PKR_MAP;
> >>>+			raster_config |= S_028350_PKR_MAP(se0_pkr_map);
> >>>+			raster_config &= C_028350_SE_MAP;
> >>>+			raster_config |= S_028350_SE_MAP(se_map);
> >>>+		}
> >>
> >>Taking a closer look again at the kernel code and register spec, I'm
> >>afraid this logic is too static. I came up with the attached
> >>incremental patch. It tries to only modify raster_config as
> >>necessary, i.e. only if there are two SEs / packers per SE / RBs per
> >>packer but one of them is disabled. The result may be different
> >>between SEs.
> >
> >Does this mean that different values of R_028350_PA_SC_RASTER_CONFIG will
> >be written depending on the SE?
> 
> Yes.
> 
> >My understanding was that this register stored the information for both
> >SEs, so the same value would be used for> SE0 and SE1.
> 
> The PKR_MAP and RB_MAP_PKR0/1 fields could only be the same for both
> SEs if the pattern of disabled RBs is always symmetric between the
> SEs (is it?), and the description of PKR_MAP in the register spec
> explicitly says 'This can be unique per SE'.
> 
> The SE_MAP field value is the same for both SEs though.
> 

Thanks for explaining, I think I understand now.  I will try to test this
out on some harvested GPUs that I have.

-Tom
> 
> -- 
> Earthling Michel Dänzer            |                  http://www.amd.com
> Libre software enthusiast          |                Mesa and X developer