Re: [PATCH xf86-video-intel v3 2/2] sna: Added AYUV format support for textured and sprite video adapters.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2018-10-10 at 22:47 +0300, Ville Syrjälä wrote:
> What's this?
> 
> >  	tmp->blt   = gen9_render_composite_blt;
> >  	tmp->box   = gen9_render_composite_box;
> >  	tmp->boxes = gen9_render_composite_boxes__blt;
> > @@ -2784,6 +2804,7 @@ gen9_render_composite_spans(struct sna *sna,
> >  	tmp->base.u.gen9.wm_kernel =
> >  		GEN9_WM_KERNEL_OPACITY | !tmp->base.is_affine;
> >  
> > +	tmp->base.gen9_kernel = GEN9_WM_KERNEL_OPACITY | !tmp-
> > >base.is_affine;
> 
> And this?

Thanks for noticing! Those are remains of my own patch when I tried to
fix the flag(running out of kernels) issue myself, then after I figured
out that you have this patch already I applied it, however looks like
for some weird reason, I didn't remove my own duplicate changes.

> 
> >  	tmp->box   = gen9_render_composite_spans_box;
> >  	tmp->boxes = gen9_render_composite_spans_boxes;
> >  	if (tmp->emit_boxes)
> > @@ -3853,6 +3874,8 @@ static void gen9_emit_video_state(struct sna
> > *sna,
> >  			src_surf_format[0] =
> > SURFACEFORMAT_B8G8R8X8_UNORM;
> >  		else if (frame->id == FOURCC_UYVY)
> >  			src_surf_format[0] =
> > SURFACEFORMAT_YCRCB_SWAPY;
> > +		else if (is_ayuv_fourcc(frame->id))
> 
> Just frame->id == FOURCC_AYUV?
> 
> > +			src_surf_format[0] =
> > SURFACEFORMAT_B8G8R8A8_UNORM;
> >  		else
> >  			src_surf_format[0] =
> > SURFACEFORMAT_YCRCB_NORMAL;
> >  
> > @@ -3903,6 +3926,9 @@ static unsigned select_video_kernel(const
> > struct sna_video *video,
> >  	case FOURCC_RGB565:
> >  		return GEN9_WM_KERNEL_VIDEO_RGB;
> >  
> > +	case FOURCC_AYUV:
> > +		return GEN9_WM_KERNEL_VIDEO_PACKED_AYUV_BT601;
> 
> Missing the colorspace handling.
> 
> > +
> >  	default:
> >  		return video->colorspace ?
> >  			GEN9_WM_KERNEL_VIDEO_PACKED_BT709 :
> > @@ -4080,6 +4106,7 @@ static void gen9_render_fini(struct sna *sna)
> >  	kgem_bo_destroy(&sna->kgem, sna-
> > >render_state.gen9.general_bo);
> >  }
> >  
> > +
> 
> stray whitespace
> 
> >  static bool gen9_render_setup(struct sna *sna)
> >  {
> >  	struct gen9_render_state *state = &sna->render_state.gen9;
> > diff --git a/src/sna/sna_render.h b/src/sna/sna_render.h
> > index a4e5b56a..7ae9a0b0 100644
> > --- a/src/sna/sna_render.h
> > +++ b/src/sna/sna_render.h
> > @@ -154,6 +154,7 @@ struct sna_composite_op {
> >  			uint8_t wm_kernel;
> >  		} gen9;
> >  	} u;
> > +	unsigned long gen9_kernel;
> 
> ?
> 
> >  
> >  	void *priv;
> >  };
> > @@ -617,6 +618,9 @@ enum {
> >  	GEN9_WM_KERNEL_VIDEO_NV12_BT709,
> >  	GEN9_WM_KERNEL_VIDEO_PACKED_BT709,
> >  
> > +	GEN9_WM_KERNEL_VIDEO_PACKED_AYUV_BT601,
> > +	GEN9_WM_KERNEL_VIDEO_PACKED_AYUV_BT709,
> > +
> >  	GEN9_WM_KERNEL_VIDEO_RGB,
> >  	GEN9_WM_KERNEL_COUNT
> >  };
> > diff --git a/src/sna/sna_video.c b/src/sna/sna_video.c
> > index 55405f81..b3c2bdc6 100644
> > --- a/src/sna/sna_video.c
> > +++ b/src/sna/sna_video.c
> > @@ -281,6 +281,7 @@ sna_video_frame_set_rotation(struct sna_video
> > *video,
> >  	} else {
> >  		switch (frame->id) {
> >  		case FOURCC_RGB888:
> > +		case FOURCC_AYUV:
> >  			if (rotation & (RR_Rotate_90 |
> > RR_Rotate_270)) {
> >  				frame->pitch[0] = ALIGN((height <<
> > 2), align);
> >  				frame->size = (int)frame->pitch[0] 
> > * width;
> > @@ -584,6 +585,149 @@ sna_copy_packed_data(struct sna_video *video,
> >  	}
> >  }
> >  
> > +static void
> > +sna_copy_packed_data_ayuv(struct sna_video *video,
> 
> Maybe sna_copy_ayuv_data() ?
> 
> > +		     const struct sna_video_frame *frame,
> > +		     const uint8_t *buf,
> > +		     uint8_t *dst)
> > +{
> > +	int pitch = frame->width << 2;
> > +	const uint8_t *src, *s;
> > +	int x, y, w, h;
> > +	int i, j;
> > +
> > +	if (video->textured) {
> > +		/* XXX support copying cropped extents */
> > +		x = y = 0;
> > +		w = frame->width;
> > +		h = frame->height;
> > +	} else {
> > +		x = frame->image.x1;
> > +		y = frame->image.y1;
> > +		w = frame->image.x2 - frame->image.x1;
> > +		h = frame->image.y2 - frame->image.y1;
> > +	}
> > +
> > +	src = buf + (y * pitch) + (x << 2);
> > +	switch (frame->rotation) {
> > +	case RR_Rotate_0:
> > +		w <<= 2;
> > +		for (i = 0; i < h; i++) {
> > +			for (j = 0; j < w; j += 4) {
> > +				uint32_t reverse_dw, dw =
> > *((uint32_t*)(&src[i * frame->pitch[0] + j]));
> > +				if (!video->textured) {
> 
> Maybe just byteswap always to make things work the same way for both
> textured and sprites?
> 
> bswap_32() or something?

Definitely, I'll put this to separate function.

> 
> > +					/*
> > +					 * For textured we do byte
> > reversing in shader.
> > +					 * Have to reverse bytes
> > order, because the only
> > +					 * player which supports
> > AYUV format currently is
> > +					 * Gstreamer and it
> > supports in bad way, even though
> > +					 * spec says MSB:AYUV, we
> > get the bytes opposite way.
> > +					 */
> > +					reverse_dw = 0;
> > +					reverse_dw |= ((dw &
> > 0x000000ff) << 24);
> > +					reverse_dw |= ((dw &
> > 0x0000ff00) << 8);
> > +					reverse_dw |= ((dw &
> > 0x00ff0000) >> 8);
> > +					reverse_dw |= (dw >> 24);
> > +				}
> > +				else
> > +					reverse_dw = dw;
> > +				*((uint32_t*)&dst[i * frame-
> > >pitch[0] + j]) = reverse_dw;
> > +			}
> > +		}
> > +		break;
> > +	case RR_Rotate_90:
> > +		h <<= 2;
> > +		for (i = 0; i < h; i += 4) {
> > +			for (j = 0;j < w; j++) {
> > +				uint32_t reverse_dw, dw;
> > +				dw = 0;
> > +				dw |= (src[i * frame->pitch[0] +
> > j]);
> > +				dw |= ((uint32_t)src[(i + 1) *
> > frame->pitch[0] + j] << 8);
> > +				dw |= ((uint32_t)src[(i + 2) *
> > frame->pitch[0] + j] << 16);
> > +				dw |= ((uint32_t)src[(i + 3) *
> > frame->pitch[0] + j] << 24);
> 
> This looks dodgy.
> 
> > +				if (!video->textured) {
> > +					/*
> > +					 * For textured we do byte
> > reversing in shader.
> > +					 * Have to reverse bytes
> > order, because the only
> > +					 * player which supports
> > AYUV format currently is
> > +					 * Gstreamer and it
> > supports in bad way, even though
> > +					 * spec says MSB:AYUV, we
> > get the bytes opposite way.
> > +					 */
> > +					reverse_dw = 0;
> > +					reverse_dw |= ((dw &
> > 0x000000ff) << 24);
> > +					reverse_dw |= ((dw &
> > 0x0000ff00) << 8);
> > +					reverse_dw |= ((dw &
> > 0x00ff0000) >> 8);
> > +					reverse_dw |= (dw >> 24);
> > +				}
> > +				else
> > +					reverse_dw = dw;
> > +				*((uint32_t*)&dst[(w - j - 1) * h
> > + i]) = reverse_dw;
> > +			}
> > +		}
> > +		break;
> > +	case RR_Rotate_180:
> > +		w <<= 2;
> > +		for (i = 0; i < h; i++) {
> > +			for (j = 0;j < w; j += 4) {
> > +				uint32_t reverse_dw, dw;
> > +				dw = 0;
> > +				dw |= (src[i * frame->pitch[0] + j
> > + 3]);
> > +				dw |= ((uint32_t)src[i * frame-
> > >pitch[0] + j + 2] << 8);
> > +				dw |= ((uint32_t)src[i * frame-
> > >pitch[0] + j + 1] << 16);
> > +				dw |= ((uint32_t)src[i * frame-
> > >pitch[0]] << 24);
> > +				if (!video->textured) {
> > +					/*
> > +					 * For textured we do byte
> > reversing in shader.
> > +					 * Have to reverse bytes
> > order, because the only
> > +					 * player which supports
> > AYUV format currently is
> > +					 * Gstreamer and it
> > supports in bad way, even though
> > +					 * spec says MSB:AYUV, we
> > get the bytes opposite way.
> > +					 */
> > +					reverse_dw = 0;
> > +					reverse_dw |= ((dw &
> > 0x000000ff) << 24);
> > +					reverse_dw |= ((dw &
> > 0x0000ff00) << 8);
> > +					reverse_dw |= ((dw &
> > 0x00ff0000) >> 8);
> > +					reverse_dw |= (dw >> 24);
> > +				}
> > +				else
> > +					reverse_dw = dw;
> > +				*((uint32_t*)&dst[(h - i - 1) * w
> > + (w - j - 4)]) = reverse_dw;
> > +			}
> > +		}
> > +		break;
> > +	case RR_Rotate_270:
> > +		h <<= 2;
> > +		for (i = 0; i < h; i += 4) {
> > +			for (j = 0; j < w; j++) {
> > +				uint32_t reverse_dw, dw;
> > +				dw = 0;
> > +				dw |= (src[(i + 3) * frame-
> > >pitch[0] + j]);
> > +				dw |= ((uint32_t)src[(i + 2) *
> > frame->pitch[0] + j] << 8);
> > +				dw |= ((uint32_t)src[(i + 1) *
> > frame->pitch[0] + j] << 16);
> > +				dw |= ((uint32_t)src[i * frame-
> > >pitch[0] + j] << 24);
> > +				if (!video->textured) {
> > +					/*
> > +					 * For textured we do byte
> > reversing in shader.
> > +					 * Have to reverse bytes
> > order, because the only
> > +					 * player which supports
> > AYUV format currently is
> > +					 * Gstreamer and it
> > supports in bad way, even though
> > +					 * spec says MSB:AYUV, we
> > get the bytes opposite way.
> > +					 */
> > +					reverse_dw = 0;
> > +					reverse_dw |= ((dw &
> > 0x000000ff) << 24);
> > +					reverse_dw |= ((dw &
> > 0x0000ff00) << 8);
> > +					reverse_dw |= ((dw &
> > 0x00ff0000) >> 8);
> > +					reverse_dw |= (dw >> 24);
> > +				}
> > +				else
> > +					reverse_dw = dw;
> > +				*((uint32_t*)&dst[j * h + (h - i -
> > 4)]) = reverse_dw;
> > +			}
> > +		}
> > +		break;
> > +	}
> > +}
> > +
> >  bool
> >  sna_video_copy_data(struct sna_video *video,
> >  		    struct sna_video_frame *frame,
> > @@ -709,6 +853,9 @@ use_gtt: /* copy data, must use GTT so that we
> > keep the overlay uncached */
> >  		sna_copy_nv12_data(video, frame, buf, dst);
> >  	else if (is_planar_fourcc(frame->id))
> >  		sna_copy_planar_data(video, frame, buf, dst);
> > +	else if (is_ayuv_fourcc(frame->id))
> > +		/* Some hardcoding is done in default
> > sna_copy_packed_data, so added a specific function */
> > +		sna_copy_packed_data_ayuv(video, frame, buf, dst);
> >  	else
> >  		sna_copy_packed_data(video, frame, buf, dst);
> >  
> > diff --git a/src/sna/sna_video.h b/src/sna/sna_video.h
> > index bbd3f0fd..d18c79e5 100644
> > --- a/src/sna/sna_video.h
> > +++ b/src/sna/sna_video.h
> > @@ -39,6 +39,7 @@ THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> >  #define FOURCC_RGB565 ((16 << 24) + ('B' << 16) + ('G' << 8) +
> > 'R')
> >  #define FOURCC_RGB888 ((24 << 24) + ('B' << 16) + ('G' << 8) +
> > 'R')
> >  #define FOURCC_NV12 (('2' << 24) + ('1' << 16) + ('V' << 8) + 'N')
> > +#define FOURCC_AYUV (('V' << 24) + ('U' << 16) + ('Y' << 8) + 'A')
> >  
> >  /*
> >   * Below, a dummy picture type that is used in XvPutImage
> > @@ -79,6 +80,15 @@ THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> >  	XvTopToBottom \
> >  }
> >  
> > +#define XVIMAGE_AYUV { \
> > +	FOURCC_AYUV, XvYUV, LSBFirst, \
> > +	{'P', 'A', 'S', 'S', 'T', 'H', 'R', 'O', 'U', 'G', 'H', '
> > ', 'A', 'Y', 'U', 'V'}, \
> 
> That should be something more like
> {'A','Y','U','V',....}
> 
> > +	32, XvPacked, 1, 24, 0xff<<16, 0xff<<8, 0xff<<0, 0, 0, 0,
> > 0, 0, 0, 0, 0, 0, \
> 
> Comments in XvImageRec suggest that we shouldn't have depth/masks
> for yuv formats, and we should instead fill the yuv related things
> instead.
> 
> > +	{'V', 'U', 'Y', 'X', 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, \
> 
> 'A','Y','U','V' would match the gst byte order I guess.
> 
> > +	XvTopToBottom \
> > +}
> > +
> > +
> >  struct sna_video {
> >  	struct sna *sna;
> >  
> > @@ -189,6 +199,16 @@ static inline int is_nv12_fourcc(int id)
> >  	}
> >  }
> >  
> > +static inline int is_ayuv_fourcc(int id)
> > +{
> > +	switch (id) {
> > +	case FOURCC_AYUV:
> > +		return 1;
> > +	default:
> > +		return 0;
> > +	}
> > +}
> > +
> >  bool
> >  sna_video_clip_helper(struct sna_video *video,
> >  		      struct sna_video_frame *frame,
> > diff --git a/src/sna/sna_video_sprite.c
> > b/src/sna/sna_video_sprite.c
> > index 8b7ae8ae..288f1d62 100644
> > --- a/src/sna/sna_video_sprite.c
> > +++ b/src/sna/sna_video_sprite.c
> > @@ -47,7 +47,7 @@
> >  #define DRM_FORMAT_YUYV         fourcc_code('Y', 'U', 'Y', 'V') /*
> > [31:0] Cr0:Y1:Cb0:Y0 8:8:8:8 little endian */
> >  #define DRM_FORMAT_UYVY         fourcc_code('U', 'Y', 'V', 'Y') /*
> > [31:0] Y1:Cr0:Y0:Cb0 8:8:8:8 little endian */
> >  #define DRM_FORMAT_NV12         fourcc_code('N', 'V', '1', '2') /*
> > 2x2 subsampled Cr:Cb plane */
> > -
> > +#define DRM_FORMAT_XYUV8888     fourcc_code('X', 'Y', 'U', 'V') /*
> > 2x2 subsampled Cr:Cb plane */
> 
> Bogus comment about subsampling.
> 
> >  #define has_hw_scaling(sna, video) ((sna)->kgem.gen < 071 || \
> >  				    (sna)->kgem.gen >= 0110)
> >  
> > @@ -74,11 +74,11 @@ static Atom xvColorKey, xvAlwaysOnTop,
> > xvSyncToVblank, xvColorspace;
> >  
> >  static XvFormatRec formats[] = { {15}, {16}, {24} };
> >  static const XvImageRec images[] = { XVIMAGE_YUY2, XVIMAGE_UYVY,
> > -				     XVMC_RGB888 };
> > +				     XVMC_RGB888, XVIMAGE_AYUV };
> >  static const XvImageRec images_rgb565[] = { XVIMAGE_YUY2,
> > XVIMAGE_UYVY,
> > -					    XVMC_RGB888,
> > XVMC_RGB565 };
> > +					    XVMC_RGB888,
> > XVMC_RGB565, XVIMAGE_AYUV };
> >  static const XvImageRec images_nv12[] = { XVIMAGE_YUY2,
> > XVIMAGE_UYVY,
> > -					  XVIMAGE_NV12,
> > XVMC_RGB888, XVMC_RGB565 };
> > +					  XVIMAGE_NV12,
> > XVMC_RGB888, XVMC_RGB565, XVIMAGE_AYUV };
> 
> This will now advertize AYUV on every platform.
> 
> >  static const XvAttributeRec attribs[] = {
> >  	{ XvSettable | XvGettable, 0, 1, (char *)"XV_COLORSPACE"
> > }, /* BT.601, BT.709 */
> >  	{ XvSettable | XvGettable, 0, 0xffffff, (char
> > *)"XV_COLORKEY" },
> > @@ -364,6 +364,10 @@ sna_video_sprite_show(struct sna *sna,
> >  		case FOURCC_UYVY:
> >  			f.pixel_format = DRM_FORMAT_UYVY;
> >  			break;
> > +		case FOURCC_AYUV:
> > +			/* i915 doesn't support alpha, so we use
> > XYUV */
> > +			f.pixel_format = DRM_FORMAT_XYUV8888;
> > +			break;
> >  		case FOURCC_YUY2:
> >  		default:
> >  			f.pixel_format = DRM_FORMAT_YUYV;
> > @@ -705,7 +709,12 @@ static int
> > sna_video_sprite_query(ddQueryImageAttributes_ARGS)
> >  		tmp *= (*h >> 1);
> >  		size += tmp;
> >  		break;
> > -
> > +	case FOURCC_AYUV:
> > +		tmp = *w << 2;
> > +		if (pitches)
> > +			pitches[0] = tmp;
> > +		size = *h * tmp;
> > +		break;
> >  	default:
> >  		*w = (*w + 1) & ~1;
> >  		*h = (*h + 1) & ~1;
> > diff --git a/src/sna/sna_video_textured.c
> > b/src/sna/sna_video_textured.c
> > index a784fe2e..21c5f379 100644
> > --- a/src/sna/sna_video_textured.c
> > +++ b/src/sna/sna_video_textured.c
> > @@ -68,6 +68,8 @@ static const XvImageRec gen4_Images[] = {
> >  	XVIMAGE_I420,
> >  	XVIMAGE_NV12,
> >  	XVIMAGE_UYVY,
> > +	XVIMAGE_AYUV,
> > +	XVMC_RGB888,
> 
> RGB888 too?
> 
> >  	XVMC_YUV,
> >  };
> >  
> > @@ -337,6 +339,12 @@
> > sna_video_textured_query(ddQueryImageAttributes_ARGS)
> >  			pitches[0] = size;
> >  		size *= *h;
> >  		break;
> > +	case FOURCC_AYUV:
> > +		size = *w << 2;
> > +		if (pitches)
> > +			pitches[0] = size;
> > +		size *= *h;
> > +		break;
> >  	case FOURCC_XVMC:
> >  		*h = (*h + 1) & ~1;
> >  		size = sizeof(uint32_t);
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
-- 
Best Regards,

Lisovskiy Stanislav
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux