After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 792812 - kmsallocator: add alignment when create fb
kmsallocator: add alignment when create fb
Status: RESOLVED INVALID
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.12.x
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-01-23 06:23 UTC by Haihua Hu
Modified: 2018-08-27 19:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
kmsallocator: add alignment when create fb (2.76 KB, patch)
2018-01-23 06:25 UTC, Haihua Hu
rejected Details | Review

Description Haihua Hu 2018-01-23 06:23:30 UTC
Drm core will check this alignment when create fb
Comment 1 Haihua Hu 2018-01-23 06:24:12 UTC
in drivers/gpu/drm/drm_framebuffer.c


216         hsub = drm_format_horz_chroma_subsampling(r->pixel_format);                                                                                                                  
217         vsub = drm_format_vert_chroma_subsampling(r->pixel_format);                                                                                                                  
218         num_planes = drm_format_num_planes(r->pixel_format);                                                                                                                         
219                                                                                                                                                                                      
220         if (r->width == 0 || r->width % hsub) {                                                                                                                                      
221                 DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);                                                                                                               
222                 return -EINVAL;                                                                                                                                                      
223         }                                                                                                                                                                            
224                                                                                                                                                                                      
225         if (r->height == 0 || r->height % vsub) {                                                                                                                                    
226                 DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height);                                                                                                             
227                 return -EINVAL;                                                                                                                                                      
228         }
Comment 2 Haihua Hu 2018-01-23 06:25:28 UTC
Created attachment 367278 [details] [review]
kmsallocator: add alignment when create fb
Comment 3 Víctor Manuel Jáquez Leal 2018-01-23 09:05:52 UTC
Review of attachment 367278 [details] [review]:

::: sys/kms/gstkmsutils.c
@@ +140,3 @@
+    case DRM_FORMAT_UYVY:
+    case DRM_FORMAT_YUYV:
+    case DRM_FORMAT_YVYU:

If I understand correctly [1], DRM_FORMAT_YUYV, DRM_FORMAT_YVYU, DRM_FORMAT_UYVY, etc., have different horizontal and vertical subsampling, thus this looks wrong.




1. http://elixir.free-electrons.com/linux/latest/source/drivers/gpu/drm/drm_fourcc.c#L105
Comment 4 Haihua Hu 2018-01-23 09:48:02 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #3)
> Review of attachment 367278 [details] [review] [review]:
> 
> ::: sys/kms/gstkmsutils.c
> @@ +140,3 @@
> +    case DRM_FORMAT_UYVY:
> +    case DRM_FORMAT_YUYV:
> +    case DRM_FORMAT_YVYU:
> 
> If I understand correctly [1], DRM_FORMAT_YUYV, DRM_FORMAT_YVYU,
> DRM_FORMAT_UYVY, etc., have different horizontal and vertical subsampling,
> thus this looks wrong.
> 
> 
> 
> 
> 1.
> http://elixir.free-electrons.com/linux/latest/source/drivers/gpu/drm/
> drm_fourcc.c#L105

Oh, that's right, my linux kernel code is out of date. I need to correct it according to the latest code
Comment 5 Nicolas Dufresne (ndufresne) 2018-01-23 16:18:42 UTC
When reporting an issue, you should describe the issues that this issue causes. Otherwise, your patch is likely to be ignored (considered theoretical).

About hsub, the same information is available in GStreamer through GstVideooFormatInfo stored in GstVideoInfo. That means you can do the same math.
Comment 6 Nicolas Dufresne (ndufresne) 2018-01-24 16:40:25 UTC
Any update, is this bug report really valid after all ? Reading a bit the upstream kernel code, there is no reason to pad the width further, as the returned stride will overlay it. For packed formats, the driver is also allowed to return a larger size, regardless if the height is aligned or not.
Comment 7 Haihua Hu 2018-01-25 05:51:59 UTC
(In reply to Nicolas Dufresne (stormer) from comment #6)
> Any update, is this bug report really valid after all ? Reading a bit the
> upstream kernel code, there is no reason to pad the width further, as the
> returned stride will overlay it. For packed formats, the driver is also
> allowed to return a larger size, regardless if the height is aligned or not.

I have not investigate the latest kernel code. But it will fail when custom kernel version is low
Comment 8 Haihua Hu 2018-01-25 05:52:37 UTC
(In reply to Nicolas Dufresne (stormer) from comment #6)
> Any update, is this bug report really valid after all ? Reading a bit the
> upstream kernel code, there is no reason to pad the width further, as the
> returned stride will overlay it. For packed formats, the driver is also
> allowed to return a larger size, regardless if the height is aligned or not.

Actually my kernel version is 4.9.51
Comment 9 Nicolas Dufresne (ndufresne) 2018-08-27 19:26:30 UTC
Review of attachment 367278 [details] [review]:

It's really the kernel jobs to increase the width and height for it's own alignment requirement, and userspace job to read it back.
Comment 10 Nicolas Dufresne (ndufresne) 2018-08-27 19:27:01 UTC
Note that for stable kernel still supported, you can request appropriate backports.