GNOME Bugzilla – Bug 768446
kmssink: on intel, stride must be 64 byte aligned
Last modified: 2016-10-31 14:30:33 UTC
While testing kmssink in i965, I have found, when trying to negotiate a video with format YUY2 720x576 the addfb2 failed with an "Invalid argument" Debugging the kernel, found this message: kernel: [drm:intel_framebuffer_init] pitch (1440) must be at least 64 byte aligned kernel: [drm:internal_framebuffer_create] could not create framebuffer kernel: [drm:drm_ioctl] ret = -22 My first approach is to have two infos, one with the original video info, and a second, used to allocate, with the aligned stride. Is it a good approach? Would it work in other platforms?
Would it be possible to clarify who picked this stride ? Ideally, their should be a kernel API that provides the requires stride for a certain width. If we instead get the alignment info, we can use GstVideoAlignement + gst_video_info_align() to calculate the required stride. gst_video_frame_copy() is safe for when you can't match the buffer stride with requirement. It will handle copied with different strides.
(In reply to Nicolas Dufresne (stormer) from comment #1) > Would it be possible to clarify who picked this stride ? GstVideoInfo generate that value. AddFB2 is called in the gstkmsallocator and the stride/pitch is set with pitches[i] = GST_VIDEO_INFO_PLANE_STRIDE (vinfo, i); https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/sys/kms/gstkmsallocator.c#n327 > Ideally, their > should be a kernel API that provides the requires stride for a certain > width. Are you saying that we should not fill that value?? > If we instead get the alignment info, we can use GstVideoAlignement + > gst_video_info_align() to calculate the required stride. > gst_video_frame_copy() is safe for when you can't match the buffer stride > with requirement. It will handle copied with different strides.
(In reply to Víctor Manuel Jáquez Leal from comment #2) > > Ideally, their > > should be a kernel API that provides the requires stride for a certain > > width. > > Are you saying that we should not fill that value?? > I'm saying the kernel should provide the information needed to fill this value, or this value directly.
Created attachment 332810 [details] [review] kmssink: override stride if defined in driver Some kms drivers demands specific pitches over the ones calculated by GstVideoInfo. For example, intel driver demands strides round up 64. This patch queries the driver for the prefered pitch and overwrites it in the pool's GstVideoInfo structure.
I haven't tested the patch on any other non-intel platform. I anyone can test it, I'll appreciate it.
Created attachment 332811 [details] [review] kmssink: override stride if defined in driver Some kms drivers demands specific pitches over the ones calculated by GstVideoInfo. For example, intel driver demands strides round up 64. This patch queries the driver for the prefered pitch and overwrites it in the pool's GstVideoInfo structure.
Review of attachment 332811 [details] [review]: I haven't tested on any hardware, but from reading the patch it makes sense.
Attachment 332811 [details] pushed as c117165 - kmssink: override stride if defined in driver