From: Ville Syrjälä ville.syrjala@linux.intel.com
There will be a need for this function in drm_crtc.c later. This avoids making drm_crtc.c depend on drm_crtc_helper.c.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 32 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_crtc_helper.c | 33 --------------------------------- include/drm/drm_crtc.h | 2 ++ include/drm/drm_crtc_helper.h | 2 -- 4 files changed, 34 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d2d9dc5..a99558e 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3454,3 +3454,35 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, } } EXPORT_SYMBOL(drm_fb_get_bpp_depth); + +/** + * drm_format_num_planes - get the number of planes for format + * @format: pixel format (DRM_FORMAT_*) + * + * RETURNS: + * The number of planes used by the specified pixel format. + */ +int drm_format_num_planes(uint32_t format) +{ + switch (format) { + case DRM_FORMAT_YUV410: + case DRM_FORMAT_YVU410: + case DRM_FORMAT_YUV411: + case DRM_FORMAT_YVU411: + case DRM_FORMAT_YUV420: + case DRM_FORMAT_YVU420: + case DRM_FORMAT_YUV422: + case DRM_FORMAT_YVU422: + case DRM_FORMAT_YUV444: + case DRM_FORMAT_YVU444: + return 3; + case DRM_FORMAT_NV12: + case DRM_FORMAT_NV21: + case DRM_FORMAT_NV16: + case DRM_FORMAT_NV61: + return 2; + default: + return 1; + } +} +EXPORT_SYMBOL(drm_format_num_planes); diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index d9d6684..dc65e81 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -1017,36 +1017,3 @@ void drm_helper_hpd_irq_event(struct drm_device *dev) queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0); } EXPORT_SYMBOL(drm_helper_hpd_irq_event); - - -/** - * drm_format_num_planes - get the number of planes for format - * @format: pixel format (DRM_FORMAT_*) - * - * RETURNS: - * The number of planes used by the specified pixel format. - */ -int drm_format_num_planes(uint32_t format) -{ - switch (format) { - case DRM_FORMAT_YUV410: - case DRM_FORMAT_YVU410: - case DRM_FORMAT_YUV411: - case DRM_FORMAT_YVU411: - case DRM_FORMAT_YUV420: - case DRM_FORMAT_YVU420: - case DRM_FORMAT_YUV422: - case DRM_FORMAT_YVU422: - case DRM_FORMAT_YUV444: - case DRM_FORMAT_YVU444: - return 3; - case DRM_FORMAT_NV12: - case DRM_FORMAT_NV21: - case DRM_FORMAT_NV16: - case DRM_FORMAT_NV61: - return 2; - default: - return 1; - } -} -EXPORT_SYMBOL(drm_format_num_planes); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 9595c2c..697c072 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1023,4 +1023,6 @@ extern int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); +extern int drm_format_num_planes(uint32_t format); + #endif /* __DRM_CRTC_H__ */ diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index 37515d1..3add00e 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -145,6 +145,4 @@ extern void drm_helper_hpd_irq_event(struct drm_device *dev); extern void drm_kms_helper_poll_disable(struct drm_device *dev); extern void drm_kms_helper_poll_enable(struct drm_device *dev);
-extern int drm_format_num_planes(uint32_t format); - #endif
From: Ville Syrjälä ville.syrjala@linux.intel.com
This function returns the bytes per pixel value based on the pixel format and plane index.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 1 + 2 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index a99558e..74979ae 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3486,3 +3486,48 @@ int drm_format_num_planes(uint32_t format) } } EXPORT_SYMBOL(drm_format_num_planes); + +/** + * drm_format_plane_cpp - determine the bytes per pixel value + * @format: pixel format (DRM_FORMAT_*) + * @plane: plane index + * + * RETURNS: + * The bytes per pixel value for the specified plane. + */ +int drm_format_plane_cpp(uint32_t format, int plane) +{ + unsigned int depth; + int bpp; + + if (plane >= drm_format_num_planes(format)) + return 0; + + switch (format) { + case DRM_FORMAT_YUYV: + case DRM_FORMAT_YVYU: + case DRM_FORMAT_UYVY: + case DRM_FORMAT_VYUY: + return 2; + case DRM_FORMAT_NV12: + case DRM_FORMAT_NV21: + case DRM_FORMAT_NV16: + case DRM_FORMAT_NV61: + return plane ? 2 : 1; + case DRM_FORMAT_YUV410: + case DRM_FORMAT_YVU410: + case DRM_FORMAT_YUV411: + case DRM_FORMAT_YVU411: + case DRM_FORMAT_YUV420: + case DRM_FORMAT_YVU420: + case DRM_FORMAT_YUV422: + case DRM_FORMAT_YVU422: + case DRM_FORMAT_YUV444: + case DRM_FORMAT_YVU444: + return 1; + default: + drm_fb_get_bpp_depth(format, &depth, &bpp); + return bpp >> 3; + } +} +EXPORT_SYMBOL(drm_format_plane_cpp); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 697c072..ae7be55 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1024,5 +1024,6 @@ extern int drm_mode_destroy_dumb_ioctl(struct drm_device *dev, extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); extern int drm_format_num_planes(uint32_t format); +extern int drm_format_plane_cpp(uint32_t format, int plane);
#endif /* __DRM_CRTC_H__ */
From: Ville Syrjälä ville.syrjala@linux.intel.com
These functions return the chroma subsampling factors for the specified pixel format.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 2 + 2 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 74979ae..95f15c8 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3531,3 +3531,63 @@ int drm_format_plane_cpp(uint32_t format, int plane) } } EXPORT_SYMBOL(drm_format_plane_cpp); + +/** + * drm_format_horz_chroma_subsampling - get the horizontal chroma subsampling factor + * @format: pixel format (DRM_FORMAT_*) + * + * RETURNS: + * The horizontal chroma subsampling factor for the + * specified pixel format. + */ +int drm_format_horz_chroma_subsampling(uint32_t format) +{ + switch (format) { + case DRM_FORMAT_YUV411: + case DRM_FORMAT_YVU411: + case DRM_FORMAT_YUV410: + case DRM_FORMAT_YVU410: + return 4; + case DRM_FORMAT_YUYV: + case DRM_FORMAT_YVYU: + case DRM_FORMAT_UYVY: + case DRM_FORMAT_VYUY: + case DRM_FORMAT_NV12: + case DRM_FORMAT_NV21: + case DRM_FORMAT_NV16: + case DRM_FORMAT_NV61: + case DRM_FORMAT_YUV422: + case DRM_FORMAT_YVU422: + case DRM_FORMAT_YUV420: + case DRM_FORMAT_YVU420: + return 2; + default: + return 1; + } +} +EXPORT_SYMBOL(drm_format_horz_chroma_subsampling); + +/** + * drm_format_vert_chroma_subsampling - get the vertical chroma subsampling factor + * @format: pixel format (DRM_FORMAT_*) + * + * RETURNS: + * The vertical chroma subsampling factor for the + * specified pixel format. + */ +int drm_format_vert_chroma_subsampling(uint32_t format) +{ + switch (format) { + case DRM_FORMAT_YUV410: + case DRM_FORMAT_YVU410: + return 4; + case DRM_FORMAT_YUV420: + case DRM_FORMAT_YVU420: + case DRM_FORMAT_NV12: + case DRM_FORMAT_NV21: + return 2; + default: + return 1; + } +} +EXPORT_SYMBOL(drm_format_vert_chroma_subsampling); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index ae7be55..824ab09 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1025,5 +1025,7 @@ extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); extern int drm_format_num_planes(uint32_t format); extern int drm_format_plane_cpp(uint32_t format, int plane); +extern int drm_format_horz_chroma_subsampling(uint32_t format); +extern int drm_format_vert_chroma_subsampling(uint32_t format);
#endif /* __DRM_CRTC_H__ */
Am 05.04.2012 20:35, schrieb ville.syrjala@linux.intel.com:
From: Ville Syrjälä ville.syrjala@linux.intel.com
These functions return the chroma subsampling factors for the specified pixel format.
Hmm not really related but looking at it this reminds me these formats always look a bit underspecified wrt chroma subsample positions. Are these fixed, undefined, or what (even mpeg1 and mpeg2 differ there, and let's not even talk about the big mess that interlaced mpeg2 is)?
Roland
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_crtc.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 2 + 2 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 74979ae..95f15c8 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3531,3 +3531,63 @@ int drm_format_plane_cpp(uint32_t format, int plane) } } EXPORT_SYMBOL(drm_format_plane_cpp);
+/**
- drm_format_horz_chroma_subsampling - get the horizontal chroma subsampling factor
- @format: pixel format (DRM_FORMAT_*)
- RETURNS:
- The horizontal chroma subsampling factor for the
- specified pixel format.
- */
+int drm_format_horz_chroma_subsampling(uint32_t format) +{
- switch (format) {
- case DRM_FORMAT_YUV411:
- case DRM_FORMAT_YVU411:
- case DRM_FORMAT_YUV410:
- case DRM_FORMAT_YVU410:
return 4;- case DRM_FORMAT_YUYV:
- case DRM_FORMAT_YVYU:
- case DRM_FORMAT_UYVY:
- case DRM_FORMAT_VYUY:
- case DRM_FORMAT_NV12:
- case DRM_FORMAT_NV21:
- case DRM_FORMAT_NV16:
- case DRM_FORMAT_NV61:
- case DRM_FORMAT_YUV422:
- case DRM_FORMAT_YVU422:
- case DRM_FORMAT_YUV420:
- case DRM_FORMAT_YVU420:
return 2;- default:
return 1;- }
+} +EXPORT_SYMBOL(drm_format_horz_chroma_subsampling);
+/**
- drm_format_vert_chroma_subsampling - get the vertical chroma subsampling factor
- @format: pixel format (DRM_FORMAT_*)
- RETURNS:
- The vertical chroma subsampling factor for the
- specified pixel format.
- */
+int drm_format_vert_chroma_subsampling(uint32_t format) +{
- switch (format) {
- case DRM_FORMAT_YUV410:
- case DRM_FORMAT_YVU410:
return 4;- case DRM_FORMAT_YUV420:
- case DRM_FORMAT_YVU420:
- case DRM_FORMAT_NV12:
- case DRM_FORMAT_NV21:
return 2;- default:
return 1;- }
+} +EXPORT_SYMBOL(drm_format_vert_chroma_subsampling); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index ae7be55..824ab09 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1025,5 +1025,7 @@ extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); extern int drm_format_num_planes(uint32_t format); extern int drm_format_plane_cpp(uint32_t format, int plane); +extern int drm_format_horz_chroma_subsampling(uint32_t format); +extern int drm_format_vert_chroma_subsampling(uint32_t format);
#endif /* __DRM_CRTC_H__ */
On Wed, Apr 11, 2012 at 08:53:08PM +0200, Roland Scheidegger wrote:
Am 05.04.2012 20:35, schrieb ville.syrjala@linux.intel.com:
From: Ville Syrjälä ville.syrjala@linux.intel.com
These functions return the chroma subsampling factors for the specified pixel format.
Hmm not really related but looking at it this reminds me these formats always look a bit underspecified wrt chroma subsample positions. Are these fixed, undefined, or what (even mpeg1 and mpeg2 differ there, and let's not even talk about the big mess that interlaced mpeg2 is)?
My thinking is that the sample positions can be configured through some other means. It doesn't affect the in memory layout at all so tying it to the format isn't necessary.
This also depends on the hardware. On some hardware you can configure it quite freely, but on some you may be left with just the one option, which is often not even documented.
I sent another patch [1] some time ago which provides a helper function for drivers dealing with good hardware.
[1] http://lists.freedesktop.org/archives/dri-devel/2011-December/017585.html
Am 12.04.2012 14:23, schrieb Ville Syrjälä:
On Wed, Apr 11, 2012 at 08:53:08PM +0200, Roland Scheidegger wrote:
Am 05.04.2012 20:35, schrieb ville.syrjala@linux.intel.com:
From: Ville Syrjälä ville.syrjala@linux.intel.com
These functions return the chroma subsampling factors for the specified pixel format.
Hmm not really related but looking at it this reminds me these formats always look a bit underspecified wrt chroma subsample positions. Are these fixed, undefined, or what (even mpeg1 and mpeg2 differ there, and let's not even talk about the big mess that interlaced mpeg2 is)?
My thinking is that the sample positions can be configured through some other means. It doesn't affect the in memory layout at all so tying it to the format isn't necessary.
This also depends on the hardware. On some hardware you can configure it quite freely, but on some you may be left with just the one option, which is often not even documented.
I sent another patch [1] some time ago which provides a helper function for drivers dealing with good hardware.
[1] http://lists.freedesktop.org/archives/dri-devel/2011-December/017585.html
I guess that makes sense. As long as those "other means" are available. Not like with xv where I think drivers just default to what is probably the most likely subsample positions, so assuming mpeg2/4 progressive or something (which gets mpeg1 slightly wrong), if they even bother to care about such subtleties :-).
Roland
On Fri, Apr 13, 2012 at 04:42:04AM +0200, Roland Scheidegger wrote:
Am 12.04.2012 14:23, schrieb Ville Syrjälä:
On Wed, Apr 11, 2012 at 08:53:08PM +0200, Roland Scheidegger wrote:
Am 05.04.2012 20:35, schrieb ville.syrjala@linux.intel.com:
From: Ville Syrjälä ville.syrjala@linux.intel.com
These functions return the chroma subsampling factors for the specified pixel format.
Hmm not really related but looking at it this reminds me these formats always look a bit underspecified wrt chroma subsample positions. Are these fixed, undefined, or what (even mpeg1 and mpeg2 differ there, and let's not even talk about the big mess that interlaced mpeg2 is)?
My thinking is that the sample positions can be configured through some other means. It doesn't affect the in memory layout at all so tying it to the format isn't necessary.
This also depends on the hardware. On some hardware you can configure it quite freely, but on some you may be left with just the one option, which is often not even documented.
I sent another patch [1] some time ago which provides a helper function for drivers dealing with good hardware.
[1] http://lists.freedesktop.org/archives/dri-devel/2011-December/017585.html
I guess that makes sense. As long as those "other means" are available. Not like with xv where I think drivers just default to what is probably the most likely subsample positions, so assuming mpeg2/4 progressive or something (which gets mpeg1 slightly wrong), if they even bother to care about such subtleties :-).
Yeah. I think a plane property is the best choice here. If the hardware doesn't allow you to configure this stuff, the property would just indicate the fixed configuration that the hardware supports. But this is again another case where we really want standardised and well defined properties.
From: Ville Syrjälä ville.syrjala@linux.intel.com
Perform some basic sanity check on some of the parameters in drm_mode_fb_cmd2.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 47 ++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 95f15c8..e1c8ffb 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2173,6 +2173,47 @@ static int format_check(struct drm_mode_fb_cmd2 *r) } }
+static int framebuffer_check(struct drm_mode_fb_cmd2 *r) +{ + int ret, hsub, vsub, num_planes, i; + + ret = format_check(r); + if (ret) { + DRM_ERROR("bad framebuffer format 0x%08x\n", r->pixel_format); + return ret; + } + + hsub = drm_format_horz_chroma_subsampling(r->pixel_format); + vsub = drm_format_vert_chroma_subsampling(r->pixel_format); + num_planes = drm_format_num_planes(r->pixel_format); + + if (r->width == 0 || r->width % hsub) { + DRM_ERROR("bad framebuffer width %u\n", r->height); + return -EINVAL; + } + + if (r->height == 0 || r->height % vsub) { + DRM_ERROR("bad framebuffer height %u\n", r->height); + return -EINVAL; + } + + for (i = 0; i < num_planes; i++) { + unsigned int width = r->width / (i != 0 ? hsub : 1); + + if (!r->handles[i]) { + DRM_ERROR("no buffer object handle for plane %d\n", i); + return -EINVAL; + } + + if (r->pitches[i] < drm_format_plane_cpp(r->pixel_format, i) * width) { + DRM_ERROR("bad pitch %u for plane %d\n", r->pitches[i], i); + return -EINVAL; + } + } + + return 0; +} + /** * drm_mode_addfb2 - add an FB to the graphics configuration * @inode: inode from the ioctl @@ -2212,11 +2253,9 @@ int drm_mode_addfb2(struct drm_device *dev, return -EINVAL; }
- ret = format_check(r); - if (ret) { - DRM_ERROR("bad framebuffer format 0x%08x\n", r->pixel_format); + ret = framebuffer_check(r); + if (ret) return ret; - }
mutex_lock(&dev->mode_config.mutex);
On Thu, Apr 5, 2012 at 7:35 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
There will be a need for this function in drm_crtc.c later. This avoids making drm_crtc.c depend on drm_crtc_helper.c.
Hi Ville,
I've applied these 4 patches, but I might have lost some others for you, let me know if there is some other stuff I should be reviewing for -next.
Dave.
On Fri, Apr 20, 2012 at 12:43:03PM +0100, Dave Airlie wrote:
On Thu, Apr 5, 2012 at 7:35 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
There will be a need for this function in drm_crtc.c later. This avoids making drm_crtc.c depend on drm_crtc_helper.c.
Hi Ville,
I've applied these 4 patches, but I might have lost some others for you, let me know if there is some other stuff I should be reviewing for -next.
IIRC only one patch fell through the cracks: Subject: [PATCH] drm: Unify and fix idr error handling Message-Id: 1331834311-30757-1-git-send-email-ville.syrjala@linux.intel.com
BTW you never gave any statement wrt. removing NV12M and YUV420M from drm_fourcc.h. I can send a patch if you agree, and if not, well then someone has to actually change the code to treat them exactly the same as NV12 and YUV420.
On Fri, Apr 20, 2012 at 1:23 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, Apr 20, 2012 at 12:43:03PM +0100, Dave Airlie wrote:
On Thu, Apr 5, 2012 at 7:35 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
There will be a need for this function in drm_crtc.c later. This avoids making drm_crtc.c depend on drm_crtc_helper.c.
Hi Ville,
I've applied these 4 patches, but I might have lost some others for you, let me know if there is some other stuff I should be reviewing for -next.
IIRC only one patch fell through the cracks: Subject: [PATCH] drm: Unify and fix idr error handling Message-Id: 1331834311-30757-1-git-send-email-ville.syrjala@linux.intel.com
Thanks I'll take a look at that,
BTW you never gave any statement wrt. removing NV12M and YUV420M from drm_fourcc.h. I can send a patch if you agree, and if not, well then someone has to actually change the code to treat them exactly the same as NV12 and YUV420.
I'm probably not qualified, if you and jbarnes and say one other non-Intel person agree, then send the patch with R-b and I'll apply it.
Dave.
On Fri, Apr 20, 2012 at 7:29 AM, Dave Airlie airlied@gmail.com wrote:
On Fri, Apr 20, 2012 at 1:23 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, Apr 20, 2012 at 12:43:03PM +0100, Dave Airlie wrote:
On Thu, Apr 5, 2012 at 7:35 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
There will be a need for this function in drm_crtc.c later. This avoids making drm_crtc.c depend on drm_crtc_helper.c.
Hi Ville,
I've applied these 4 patches, but I might have lost some others for you, let me know if there is some other stuff I should be reviewing for -next.
IIRC only one patch fell through the cracks: Subject: [PATCH] drm: Unify and fix idr error handling Message-Id: 1331834311-30757-1-git-send-email-ville.syrjala@linux.intel.com
Thanks I'll take a look at that,
BTW you never gave any statement wrt. removing NV12M and YUV420M from drm_fourcc.h. I can send a patch if you agree, and if not, well then someone has to actually change the code to treat them exactly the same as NV12 and YUV420.
I'm probably not qualified, if you and jbarnes and say one other non-Intel person agree, then send the patch with R-b and I'll apply it.
I agree that they seem like the same thing (as their non-M counterparts) to me.. maybe the one exception is if there was hw that somehow *only* supported discontiguous multi-planar. I can't really tell if this is the case in current exynos, it seems to only advertise XRGB8888 (but possibly I'm looking in the wrong place).
For drivers that have weird requirements, I'd suggest they use the non 7-bit ascii space (ie. one or more of bytes in fourcc is non-ascii, so as to not conflict with potential future standard fourcc's) driver specific format values.
BR, -R
Dave. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Apr 20, 2012 at 07:48:07AM -0500, Rob Clark wrote:
On Fri, Apr 20, 2012 at 7:29 AM, Dave Airlie airlied@gmail.com wrote:
On Fri, Apr 20, 2012 at 1:23 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, Apr 20, 2012 at 12:43:03PM +0100, Dave Airlie wrote:
On Thu, Apr 5, 2012 at 7:35 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
There will be a need for this function in drm_crtc.c later. This avoids making drm_crtc.c depend on drm_crtc_helper.c.
Hi Ville,
I've applied these 4 patches, but I might have lost some others for you, let me know if there is some other stuff I should be reviewing for -next.
IIRC only one patch fell through the cracks: Subject: [PATCH] drm: Unify and fix idr error handling Message-Id: 1331834311-30757-1-git-send-email-ville.syrjala@linux.intel.com
Thanks I'll take a look at that,
BTW you never gave any statement wrt. removing NV12M and YUV420M from drm_fourcc.h. I can send a patch if you agree, and if not, well then someone has to actually change the code to treat them exactly the same as NV12 and YUV420.
I'm probably not qualified, if you and jbarnes and say one other non-Intel person agree, then send the patch with R-b and I'll apply it.
I agree that they seem like the same thing (as their non-M counterparts) to me.. maybe the one exception is if there was hw that somehow *only* supported discontiguous multi-planar.
The way things are currently, anyone can feed the driver either contiguous or discontiguous planes using either format.
As a hint to userspace the M version might work, except it still doesn't tell you anything on how you need to allocate or align the planes. Since memory allocation is driver specific anyway, I see no point in overloading the pixel formats with that information. Some other mechanism to communicate such information would be needed if there is a desire to make it generic.
I can't really tell if this is the case in current exynos, it seems to only advertise XRGB8888 (but possibly I'm looking in the wrong place).
For drivers that have weird requirements, I'd suggest they use the non 7-bit ascii space (ie. one or more of bytes in fourcc is non-ascii, so as to not conflict with potential future standard fourcc's) driver specific format values.
We could define a DRM_FORMAT_DRIVER_SPECIFIC bit just like we have DRM_FORMAT_BIG_ENDIAN. We still have three bits to choose from. OTOH I'm not too worried about standard fourccs since we already differ from the standard names anyway.
On Fri, Apr 20, 2012 at 8:49 AM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, Apr 20, 2012 at 07:48:07AM -0500, Rob Clark wrote:
On Fri, Apr 20, 2012 at 7:29 AM, Dave Airlie airlied@gmail.com wrote:
On Fri, Apr 20, 2012 at 1:23 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, Apr 20, 2012 at 12:43:03PM +0100, Dave Airlie wrote:
On Thu, Apr 5, 2012 at 7:35 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
There will be a need for this function in drm_crtc.c later. This avoids making drm_crtc.c depend on drm_crtc_helper.c.
Hi Ville,
I've applied these 4 patches, but I might have lost some others for you, let me know if there is some other stuff I should be reviewing for -next.
IIRC only one patch fell through the cracks: Subject: [PATCH] drm: Unify and fix idr error handling Message-Id: 1331834311-30757-1-git-send-email-ville.syrjala@linux.intel.com
Thanks I'll take a look at that,
BTW you never gave any statement wrt. removing NV12M and YUV420M from drm_fourcc.h. I can send a patch if you agree, and if not, well then someone has to actually change the code to treat them exactly the same as NV12 and YUV420.
I'm probably not qualified, if you and jbarnes and say one other non-Intel person agree, then send the patch with R-b and I'll apply it.
I agree that they seem like the same thing (as their non-M counterparts) to me.. maybe the one exception is if there was hw that somehow *only* supported discontiguous multi-planar.
The way things are currently, anyone can feed the driver either contiguous or discontiguous planes using either format.
As a hint to userspace the M version might work, except it still doesn't tell you anything on how you need to allocate or align the planes. Since memory allocation is driver specific anyway, I see no point in overloading the pixel formats with that information. Some other mechanism to communicate such information would be needed if there is a desire to make it generic.
I can't really tell if this is the case in current exynos, it seems to only advertise XRGB8888 (but possibly I'm looking in the wrong place).
For drivers that have weird requirements, I'd suggest they use the non 7-bit ascii space (ie. one or more of bytes in fourcc is non-ascii, so as to not conflict with potential future standard fourcc's) driver specific format values.
We could define a DRM_FORMAT_DRIVER_SPECIFIC bit just like we have
yeah, that is a good idea
DRM_FORMAT_BIG_ENDIAN. We still have three bits to choose from. OTOH I'm not too worried about standard fourccs since we already differ from the standard names anyway.
well, was more just thinking from the point of view of clashes if we add more standard names later but some driver somewhere was already using that new fourcc name
Possibly I'm over-thinking this.. but seemed like a reasonable thing to separate standard and non-standard formats before a bunch of driver specific formats start cropping up.
BR, -R
-- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
2012년 4월 20일 오후 11:22, Rob Clark rob.clark@linaro.org님의 말:
On Fri, Apr 20, 2012 at 8:49 AM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, Apr 20, 2012 at 07:48:07AM -0500, Rob Clark wrote:
On Fri, Apr 20, 2012 at 7:29 AM, Dave Airlie airlied@gmail.com wrote:
On Fri, Apr 20, 2012 at 1:23 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, Apr 20, 2012 at 12:43:03PM +0100, Dave Airlie wrote:
On Thu, Apr 5, 2012 at 7:35 PM, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä ville.syrjala@linux.intel.com > > There will be a need for this function in drm_crtc.c later. This > avoids making drm_crtc.c depend on drm_crtc_helper.c.
Hi Ville,
I've applied these 4 patches, but I might have lost some others for you, let me know if there is some other stuff I should be reviewing for -next.
IIRC only one patch fell through the cracks: Subject: [PATCH] drm: Unify and fix idr error handling Message-Id: 1331834311-30757-1-git-send-email-ville.syrjala@linux.intel.com
Thanks I'll take a look at that,
BTW you never gave any statement wrt. removing NV12M and YUV420M from drm_fourcc.h. I can send a patch if you agree, and if not, well then someone has to actually change the code to treat them exactly the same as NV12 and YUV420.
I'm probably not qualified, if you and jbarnes and say one other non-Intel person agree, then send the patch with R-b and I'll apply it.
I agree that they seem like the same thing (as their non-M counterparts) to me.. maybe the one exception is if there was hw that somehow *only* supported discontiguous multi-planar.
The way things are currently, anyone can feed the driver either contiguous or discontiguous planes using either format.
As a hint to userspace the M version might work, except it still doesn't tell you anything on how you need to allocate or align the planes. Since memory allocation is driver specific anyway, I see no point in overloading the pixel formats with that information. Some other mechanism to communicate such information would be needed if there is a desire to make it generic.
I can't really tell if this is the case in current exynos, it seems to only advertise XRGB8888 (but possibly I'm looking in the wrong place).
For drivers that have weird requirements, I'd suggest they use the non 7-bit ascii space (ie. one or more of bytes in fourcc is non-ascii, so as to not conflict with potential future standard fourcc's) driver specific format values.
We could define a DRM_FORMAT_DRIVER_SPECIFIC bit just like we have
yeah, that is a good idea
DRM_FORMAT_BIG_ENDIAN. We still have three bits to choose from. OTOH I'm not too worried about standard fourccs since we already differ from the standard names anyway.
well, was more just thinking from the point of view of clashes if we add more standard names later but some driver somewhere was already using that new fourcc name
Possibly I'm over-thinking this.. but seemed like a reasonable thing to separate standard and non-standard formats before a bunch of driver specific formats start cropping up.
BR, -R
we just wanted to use multi-planar format in same way as v4l2 side and I am not still sure that it's good way to add some codes to identify them. anyway for now, I'm ok.
Thanks, Inki Dae.
-- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Dave Airlie -
InKi Dae -
Rob Clark -
Roland Scheidegger -
Ville Syrjälä -
ville.syrjala＠linux.intel.com