Ilia Mirkin
2015-Sep-07 19:55 UTC
[Nouveau] [PATCH mesa 2/3] nv30: Fix color resolving for nv3x cards
May I ask why you're doing 512x512 instead of 1024x1024? These are already scaled up coordinates, so 1024x1024 should work no? Or is it because of the seams on the edges? Do those not also appear with 512x512 or does it sample outside of the box? Separately, why not use this approach on nv40 as well? I can't imagine the blitter would be faster... does this result in lower quality? On Mon, Sep 7, 2015 at 3:50 PM, Hans de Goede <hdegoede at redhat.com> wrote:> We do not have a generic blitter on nv3x cards, so we must use the > sifm object for color resolving. > > This commit divides the sources and dest surfaces in to tiles which > match the constraints of the sifm object, so that color resolving > will work properly on nv3x cards. > > Signed-off-by: Hans de Goede <hdegoede at redhat.com> > --- > src/gallium/drivers/nouveau/nv30/nv30_miptree.c | 55 ++++++++++++++++++++++++- > 1 file changed, 54 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c > index 76bb8b8..c240030 100644 > --- a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c > +++ b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c > @@ -149,6 +149,56 @@ static void > nv30_resource_resolve(struct nv30_context *nv30, > const struct pipe_blit_info *info) > { > + struct nv30_miptree *src_mt = nv30_miptree(info->src.resource); > + struct nv30_rect src, dst; > + unsigned x, x0, x1, y, y1, w, h; > + > + define_rect(info->src.resource, 0, info->src.box.z, info->src.box.x, > + info->src.box.y, info->src.box.width, info->src.box.height, &src); > + define_rect(info->dst.resource, 0, info->dst.box.z, info->dst.box.x, > + info->dst.box.y, info->dst.box.width, info->dst.box.height, &dst); > + > + x0 = src.x0; > + x1 = src.x1; > + y1 = src.y1; > + > + /* On nv3x we must use sifm which is restricted to 512x512 tiles */ > + for (y = src.y0; y < y1; y += h) { > + h = y1 - y; > + if (h > 512) > + h = 512; > + > + src.y0 = 0; > + src.y1 = h; > + src.h = h; > + > + dst.y1 = dst.y0 + (h >> src_mt->ms_y); > + dst.h = h >> src_mt->ms_y; > + > + for (x = x0; x < x1; x += w) { > + w = x1 - x; > + if (w > 512) > + w = 512; > + > + src.offset = y * src.pitch + x * src.cpp; > + src.x0 = 0; > + src.x1 = w; > + src.w = w; > + > + dst.offset = (y >> src_mt->ms_y) * dst.pitch + > + (x >> src_mt->ms_x) * dst.cpp; > + dst.x1 = dst.x0 + (w >> src_mt->ms_x); > + dst.w = w >> src_mt->ms_x; > + > + nv30_transfer_rect(nv30, BILINEAR, &src, &dst); > + } > + } > +} > + > +static void > +nv40_resource_resolve(struct nv30_context *nv30, > + const struct pipe_blit_info *info) > +{ > struct nv30_rect src, dst; > > define_rect(info->src.resource, 0, info->src.box.z, info->src.box.x, > @@ -170,7 +220,10 @@ nv30_blit(struct pipe_context *pipe, > info.dst.resource->nr_samples <= 1 && > !util_format_is_depth_or_stencil(info.src.resource->format) && > !util_format_is_pure_integer(info.src.resource->format)) { > - nv30_resource_resolve(nv30, blit_info); > + if (nv30->screen->eng3d->oclass >= NV40_3D_CLASS) > + nv40_resource_resolve(nv30, blit_info); > + else > + nv30_resource_resolve(nv30, blit_info); > return; > } > > -- > 2.4.3 >
Hans de Goede
2015-Sep-08 07:53 UTC
[Nouveau] [PATCH mesa 2/3] nv30: Fix color resolving for nv3x cards
Hi, On 07-09-15 21:55, Ilia Mirkin wrote:> May I ask why you're doing 512x512 instead of 1024x1024? These are > already scaled up coordinates, so 1024x1024 should work no? Or is it > because of the seams on the edges? Do those not also appear with > 512x512 or does it sample outside of the box?This is my bad because of the bug fixed by patch 1/3 I had 512x512 in there for a while, I've moved back and forth between 512 and 1024 a couple of times. I've also tried 2048 but the hardware does not like that. I will retest with 1024 and submit a fixed version. I've not noticed any seams on the edges, even though I've been actively looking for them.> Separately, why not use this approach on nv40 as well? I can't imagine > the blitter would be faster... does this result in lower quality?I've the feeling the sifm bilinear filtering is more "blurry" then the blitter one. I will do some more objective (ahem) tests on nv4x and get back to you on this. Regards, Hans> > On Mon, Sep 7, 2015 at 3:50 PM, Hans de Goede <hdegoede at redhat.com> wrote: >> We do not have a generic blitter on nv3x cards, so we must use the >> sifm object for color resolving. >> >> This commit divides the sources and dest surfaces in to tiles which >> match the constraints of the sifm object, so that color resolving >> will work properly on nv3x cards. >> >> Signed-off-by: Hans de Goede <hdegoede at redhat.com> >> --- >> src/gallium/drivers/nouveau/nv30/nv30_miptree.c | 55 ++++++++++++++++++++++++- >> 1 file changed, 54 insertions(+), 1 deletion(-) >> >> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c >> index 76bb8b8..c240030 100644 >> --- a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c >> +++ b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c >> @@ -149,6 +149,56 @@ static void >> nv30_resource_resolve(struct nv30_context *nv30, >> const struct pipe_blit_info *info) >> { >> + struct nv30_miptree *src_mt = nv30_miptree(info->src.resource); >> + struct nv30_rect src, dst; >> + unsigned x, x0, x1, y, y1, w, h; >> + >> + define_rect(info->src.resource, 0, info->src.box.z, info->src.box.x, >> + info->src.box.y, info->src.box.width, info->src.box.height, &src); >> + define_rect(info->dst.resource, 0, info->dst.box.z, info->dst.box.x, >> + info->dst.box.y, info->dst.box.width, info->dst.box.height, &dst); >> + >> + x0 = src.x0; >> + x1 = src.x1; >> + y1 = src.y1; >> + >> + /* On nv3x we must use sifm which is restricted to 512x512 tiles */ >> + for (y = src.y0; y < y1; y += h) { >> + h = y1 - y; >> + if (h > 512) >> + h = 512; >> + >> + src.y0 = 0; >> + src.y1 = h; >> + src.h = h; >> + >> + dst.y1 = dst.y0 + (h >> src_mt->ms_y); >> + dst.h = h >> src_mt->ms_y; >> + >> + for (x = x0; x < x1; x += w) { >> + w = x1 - x; >> + if (w > 512) >> + w = 512; >> + >> + src.offset = y * src.pitch + x * src.cpp; >> + src.x0 = 0; >> + src.x1 = w; >> + src.w = w; >> + >> + dst.offset = (y >> src_mt->ms_y) * dst.pitch + >> + (x >> src_mt->ms_x) * dst.cpp; >> + dst.x1 = dst.x0 + (w >> src_mt->ms_x); >> + dst.w = w >> src_mt->ms_x; >> + >> + nv30_transfer_rect(nv30, BILINEAR, &src, &dst); >> + } >> + } >> +} >> + >> +static void >> +nv40_resource_resolve(struct nv30_context *nv30, >> + const struct pipe_blit_info *info) >> +{ >> struct nv30_rect src, dst; >> >> define_rect(info->src.resource, 0, info->src.box.z, info->src.box.x, >> @@ -170,7 +220,10 @@ nv30_blit(struct pipe_context *pipe, >> info.dst.resource->nr_samples <= 1 && >> !util_format_is_depth_or_stencil(info.src.resource->format) && >> !util_format_is_pure_integer(info.src.resource->format)) { >> - nv30_resource_resolve(nv30, blit_info); >> + if (nv30->screen->eng3d->oclass >= NV40_3D_CLASS) >> + nv40_resource_resolve(nv30, blit_info); >> + else >> + nv30_resource_resolve(nv30, blit_info); >> return; >> } >> >> -- >> 2.4.3 >>
Hans de Goede
2015-Sep-09 13:42 UTC
[Nouveau] [PATCH mesa 2/3] nv30: Fix color resolving for nv3x cards
Hi, On 08-09-15 09:53, Hans de Goede wrote:> Hi, > > On 07-09-15 21:55, Ilia Mirkin wrote: >> May I ask why you're doing 512x512 instead of 1024x1024? These are >> already scaled up coordinates, so 1024x1024 should work no? Or is it >> because of the seams on the edges? Do those not also appear with >> 512x512 or does it sample outside of the box? > > This is my bad because of the bug fixed by patch 1/3 I had 512x512 in there > for a while, I've moved back and forth between 512 and 1024 a couple of times. > > I've also tried 2048 but the hardware does not like that. > > I will retest with 1024 and submit a fixed version. > > I've not noticed any seams on the edges, even though I've been actively looking > for them. > >> Separately, why not use this approach on nv40 as well? I can't imagine >> the blitter would be faster... does this result in lower quality? > > I've the feeling the sifm bilinear filtering is more "blurry" then the > blitter one. I will do some more objective (ahem) tests on nv4x and get > back to you on this.Ok I've run some tests comparing the rendering between using the blitter and the sifm and the results seem identical, so one v2 using 1024x1024 blocks and doing so on all nv3x/nv4x cards coming up. Regards, Hans