Re: [Mesa-dev] [PATCH] i965: Fix bugs in intel_from_planar
On Mon, Feb 12, 2018 at 9:45 AM, Daniel Stone wrote: > Hi, > > On 12 February 2018 at 17:16, Jason Ekstrand wrote: > > On Mon, Feb 12, 2018 at 6:36 AM, Daniel Stone > wrote: > >> On further thought, I don't see how this could work at all. If we call > >> fromPlanar on plane 0 of a non-planar image with an aux plane, we need > >> to land in the first branch (due to the assert). But since plane 0 is > >> strictly less than the number of planes 1, we don't. > >> > >> I think something like this: https://hastebin.com/axuqorenax > > > > I like that. Mind making a proper patch out of it (not based on this > one) > > and sending to the list? Between the four of us, maybe we can write this > > correctly. :-) > > Oh yeah. I'm just waiting for my repo to get reactivated for CI > runners, so I can find out what I broke with that strawman. :) > Eh. Just send it to the list. I can run it through CI. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Fix bugs in intel_from_planar
Hi, On 12 February 2018 at 17:16, Jason Ekstrand wrote: > On Mon, Feb 12, 2018 at 6:36 AM, Daniel Stone wrote: >> On further thought, I don't see how this could work at all. If we call >> fromPlanar on plane 0 of a non-planar image with an aux plane, we need >> to land in the first branch (due to the assert). But since plane 0 is >> strictly less than the number of planes 1, we don't. >> >> I think something like this: https://hastebin.com/axuqorenax > > I like that. Mind making a proper patch out of it (not based on this one) > and sending to the list? Between the four of us, maybe we can write this > correctly. :-) Oh yeah. I'm just waiting for my repo to get reactivated for CI runners, so I can find out what I broke with that strawman. :) Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Fix bugs in intel_from_planar
On Mon, Feb 12, 2018 at 6:36 AM, Daniel Stone wrote: > Hi, > > On 10 February 2018 at 06:50, Jason Ekstrand wrote: > > -} else if (parent->planar_format == NULL) { > > + > > +const struct intel_image_format *f = parent->planar_format; > > +const int nplanes = f ? f->nplanes : 1; > > + > > +if (plane > nplanes) { > > + if (parent->modifier == DRM_FORMAT_MOD_INVALID) > > + return NULL; > > + > > const bool is_aux = > >isl_drm_modifier_has_aux(parent->modifier) && plane == 1; > > if (!is_aux) > > @@ -1332,10 +1338,6 @@ intel_from_planar(__DRIimage *parent, int plane, > void *loaderPrivate) > > } else { > > /* Planar formats don't support aux buffers/images */ > > assert(!isl_drm_modifier_has_aux(parent->modifier)); > > - f = parent->planar_format; > > - > > - if (plane >= f->nplanes) > > - return NULL; > > On further thought, I don't see how this could work at all. If we call > fromPlanar on plane 0 of a non-planar image with an aux plane, we need > to land in the first branch (due to the assert). But since plane 0 is > strictly less than the number of planes 1, we don't. > > I think something like this: https://hastebin.com/axuqorenax > I like that. Mind making a proper patch out of it (not based on this one) and sending to the list? Between the four of us, maybe we can write this correctly. :-) --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Fix bugs in intel_from_planar
Hi, On 10 February 2018 at 06:50, Jason Ekstrand wrote: > -} else if (parent->planar_format == NULL) { > + > +const struct intel_image_format *f = parent->planar_format; > +const int nplanes = f ? f->nplanes : 1; > + > +if (plane > nplanes) { > + if (parent->modifier == DRM_FORMAT_MOD_INVALID) > + return NULL; > + > const bool is_aux = >isl_drm_modifier_has_aux(parent->modifier) && plane == 1; > if (!is_aux) > @@ -1332,10 +1338,6 @@ intel_from_planar(__DRIimage *parent, int plane, void > *loaderPrivate) > } else { > /* Planar formats don't support aux buffers/images */ > assert(!isl_drm_modifier_has_aux(parent->modifier)); > - f = parent->planar_format; > - > - if (plane >= f->nplanes) > - return NULL; On further thought, I don't see how this could work at all. If we call fromPlanar on plane 0 of a non-planar image with an aux plane, we need to land in the first branch (due to the assert). But since plane 0 is strictly less than the number of planes 1, we don't. I think something like this: https://hastebin.com/axuqorenax Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Fix bugs in intel_from_planar
Hi Jason, On 10 February 2018 at 06:50, Jason Ekstrand wrote: > intel_from_planar(__DRIimage *parent, int plane, void *loaderPrivate) > { > int width, height, offset, stride, dri_format, index; > -const struct intel_image_format *f; > __DRIimage *image; > > -if (parent == NULL) { > +if (parent == NULL) > return NULL; > -} else if (parent->planar_format == NULL) { > + > +const struct intel_image_format *f = parent->planar_format; > +const int nplanes = f ? f->nplanes : 1; > + > +if (plane > nplanes) { This should be >= ... > + if (parent->modifier == DRM_FORMAT_MOD_INVALID) > + return NULL; > + > const bool is_aux = >isl_drm_modifier_has_aux(parent->modifier) && plane == 1; Or this will never be true. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev