diff mbox

[i-g-t,2/6] i-g-t: lib: Add plane pixel format support

Message ID 1513158652-8912-3-git-send-email-vidya.srinivas@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vidya Srinivas Dec. 13, 2017, 9:50 a.m. UTC 
From: Mahesh Kumar <mahesh1.kumar@intel.com>

This patch adds the following:
- Query supported pixel formats from kernel for a given
plane.
- Get number of supported pixel formats for a plane
- Check if format is supported by cairo
- Add support to get format name string

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 lib/igt_fb.c  | 16 ++++++++++++++++
 lib/igt_kms.c | 29 +++++++++++++++++++++++++++++
 lib/igt_kms.h |  7 +++++++
 3 files changed, 52 insertions(+)

Comments

Maarten Lankhorst Jan. 9, 2018, 12:10 p.m. UTC | #1 
Op 13-12-17 om 10:50 schreef Vidya Srinivas:
> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>
> This patch adds the following:
> - Query supported pixel formats from kernel for a given
> plane.
> - Get number of supported pixel formats for a plane
> - Check if format is supported by cairo
> - Add support to get format name string
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  lib/igt_fb.c  | 16 ++++++++++++++++
>  lib/igt_kms.c | 29 +++++++++++++++++++++++++++++
>  lib/igt_kms.h |  7 +++++++
>  3 files changed, 52 insertions(+)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d4eaed7..21d666d 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -68,6 +68,22 @@ static struct format_desc_struct {
>  	DF(XRGB2101010,	RGB30,		32, 30),
>  	DF(ARGB8888,	ARGB32,		32, 32),
>  };
> +
> +const char *igt_get_format_name(uint32_t format)
> +{
> +	switch(format) {
> +		case DRM_FORMAT_RGB565:
> +			return "RGB565";
> +		case DRM_FORMAT_XRGB8888:
> +			return "XRGB8888";
> +		case DRM_FORMAT_XRGB2101010:
> +			return "XRGB2101010";
> +		case DRM_FORMAT_ARGB8888:
> +			return "ARGB8888";
> +		default:
> +			return "Unknown";
> +	}
> +}
>  #undef DF
Right above this function, we have a struct format_desc_struct, which has a drm format parameter, and a name parameter.
Right below this function, there's a for_each_format() defined which iterates over the formats defined in this array. :)

I think igt_get_format_name can be implemented with both, removing some duplication.
>  #define for_each_format(f)	\
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 223dbe4..cc17f5c 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -3639,3 +3639,32 @@ uint32_t kmstest_get_vbl_flag(uint32_t pipe_id)
>  		return pipe_flag;
>  	}
>  }
> +
> +/**
> + * igt_is_cairo_supported_format:
> + * @pixel_format: pixel format to be checked in supported cairo list.
> + *
> + * Returns true if pixel format is present in the supported cairo pixel
> + * format list and returns false if not present/supported in cairo.
> + */
> +bool igt_is_cairo_supported_format(uint32_t pixel_format)
> +{
> +	const uint32_t *igt_formats;
> +	int num_igt_formats;
> +	int i;
> +
> +	igt_get_all_cairo_formats(&igt_formats, &num_igt_formats);
> +	for (i = 0; i < num_igt_formats; i++) {
> +		if (pixel_format == igt_formats[i])
> +			return true;
> +	}
> +	return false;
> +}
I think this function belongs to igt_fb as igt_fb_is_cairo_supported_format.
> +int igt_get_format_count_plane(igt_plane_t *plane)
> +{
> +	if (plane)
> +		return plane->drm_plane->count_formats;
> +
> +	return -EINVAL;
> +}
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 2a480bf..ced7d73 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -389,6 +389,10 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>  void igt_wait_for_vblank_count(int drm_fd, enum pipe pipe, int count);
>  
> +bool igt_is_cairo_supported_format(uint32_t pixel_format);
> +int igt_get_format_count_plane(igt_plane_t *plane);
> +const char *igt_get_format_name(uint32_t format);
> +
>  static inline bool igt_output_is_connected(igt_output_t *output)
>  {
>  	/* Something went wrong during probe? */
> @@ -486,6 +490,9 @@ static inline bool igt_output_is_connected(igt_output_t *output)
>  	for (int j__ = 0; assert(igt_can_fail()), (plane) = &(display)->pipes[(pipe)].planes[j__], \
>  		     j__ < (display)->pipes[(pipe)].n_planes; j__++)
>  
> +#define for_each_format_in_plane(plane, format)	\
> +	for (int k__ = 0; (format) = plane->drm_plane->formats[k__], k__ < plane->drm_plane->count_formats; k__++)
> +
>  #define IGT_FIXED(i,f)	((i) << 16 | (f))
>  
>  /**
Maarten Lankhorst Jan. 9, 2018, 12:32 p.m. UTC | #2 
Op 09-01-18 om 13:10 schreef Maarten Lankhorst:
> Op 13-12-17 om 10:50 schreef Vidya Srinivas:
>> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>>
>> This patch adds the following:
>> - Query supported pixel formats from kernel for a given
>> plane.
>> - Get number of supported pixel formats for a plane
>> - Check if format is supported by cairo
>> - Add support to get format name string
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>>  lib/igt_fb.c  | 16 ++++++++++++++++
>>  lib/igt_kms.c | 29 +++++++++++++++++++++++++++++
>>  lib/igt_kms.h |  7 +++++++
>>  3 files changed, 52 insertions(+)
>>
>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
>> index d4eaed7..21d666d 100644
>> --- a/lib/igt_fb.c
>> +++ b/lib/igt_fb.c
>> @@ -68,6 +68,22 @@ static struct format_desc_struct {
>>  	DF(XRGB2101010,	RGB30,		32, 30),
>>  	DF(ARGB8888,	ARGB32,		32, 32),
>>  };
>> +
>> +const char *igt_get_format_name(uint32_t format)
>> +{
>> +	switch(format) {
>> +		case DRM_FORMAT_RGB565:
>> +			return "RGB565";
>> +		case DRM_FORMAT_XRGB8888:
>> +			return "XRGB8888";
>> +		case DRM_FORMAT_XRGB2101010:
>> +			return "XRGB2101010";
>> +		case DRM_FORMAT_ARGB8888:
>> +			return "ARGB8888";
>> +		default:
>> +			return "Unknown";
>> +	}
>> +}
>>  #undef DF
> Right above this function, we have a struct format_desc_struct, which has a drm format parameter, and a name parameter.
> Right below this function, there's a for_each_format() defined which iterates over the formats defined in this array. :)
>
> I think igt_get_format_name can be implemented with both, removing some duplication.
Hmm, maybe get rid of igt_fb_is_cairo_supported_format and add a try_ version of igt_get_cairo_ctx/surface?

Other macros here look too specialized, nothing wrong with accessing plane->drm_plane directly for this information.
>>  #define for_each_format(f)	\
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 223dbe4..cc17f5c 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -3639,3 +3639,32 @@ uint32_t kmstest_get_vbl_flag(uint32_t pipe_id)
>>  		return pipe_flag;
>>  	}
>>  }
>> +
>> +/**
>> + * igt_is_cairo_supported_format:
>> + * @pixel_format: pixel format to be checked in supported cairo list.
>> + *
>> + * Returns true if pixel format is present in the supported cairo pixel
>> + * format list and returns false if not present/supported in cairo.
>> + */
>> +bool igt_is_cairo_supported_format(uint32_t pixel_format)
>> +{
>> +	const uint32_t *igt_formats;
>> +	int num_igt_formats;
>> +	int i;
>> +
>> +	igt_get_all_cairo_formats(&igt_formats, &num_igt_formats);
>> +	for (i = 0; i < num_igt_formats; i++) {
>> +		if (pixel_format == igt_formats[i])
>> +			return true;
>> +	}
>> +	return false;
>> +}
> I think this function belongs to igt_fb as igt_fb_is_cairo_supported_format.
>> +int igt_get_format_count_plane(igt_plane_t *plane)
>> +{
>> +	if (plane)
>> +		return plane->drm_plane->count_formats;
>> +
>> +	return -EINVAL;
>> +}
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index 2a480bf..ced7d73 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -389,6 +389,10 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
>>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>>  void igt_wait_for_vblank_count(int drm_fd, enum pipe pipe, int count);
>>  
>> +bool igt_is_cairo_supported_format(uint32_t pixel_format);
>> +int igt_get_format_count_plane(igt_plane_t *plane);
>> +const char *igt_get_format_name(uint32_t format);
>> +
>>  static inline bool igt_output_is_connected(igt_output_t *output)
>>  {
>>  	/* Something went wrong during probe? */
>> @@ -486,6 +490,9 @@ static inline bool igt_output_is_connected(igt_output_t *output)
>>  	for (int j__ = 0; assert(igt_can_fail()), (plane) = &(display)->pipes[(pipe)].planes[j__], \
>>  		     j__ < (display)->pipes[(pipe)].n_planes; j__++)
>>  
>> +#define for_each_format_in_plane(plane, format)	\
>> +	for (int k__ = 0; (format) = plane->drm_plane->formats[k__], k__ < plane->drm_plane->count_formats; k__++)
>> +
>>  #define IGT_FIXED(i,f)	((i) << 16 | (f))
>>  
>>  /**
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d4eaed7..21d666d 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -68,6 +68,22 @@  static struct format_desc_struct {
 	DF(XRGB2101010,	RGB30,		32, 30),
 	DF(ARGB8888,	ARGB32,		32, 32),
 };
+
+const char *igt_get_format_name(uint32_t format)
+{
+	switch(format) {
+		case DRM_FORMAT_RGB565:
+			return "RGB565";
+		case DRM_FORMAT_XRGB8888:
+			return "XRGB8888";
+		case DRM_FORMAT_XRGB2101010:
+			return "XRGB2101010";
+		case DRM_FORMAT_ARGB8888:
+			return "ARGB8888";
+		default:
+			return "Unknown";
+	}
+}
 #undef DF
 
 #define for_each_format(f)	\
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 223dbe4..cc17f5c 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -3639,3 +3639,32 @@  uint32_t kmstest_get_vbl_flag(uint32_t pipe_id)
 		return pipe_flag;
 	}
 }
+
+/**
+ * igt_is_cairo_supported_format:
+ * @pixel_format: pixel format to be checked in supported cairo list.
+ *
+ * Returns true if pixel format is present in the supported cairo pixel
+ * format list and returns false if not present/supported in cairo.
+ */
+bool igt_is_cairo_supported_format(uint32_t pixel_format)
+{
+	const uint32_t *igt_formats;
+	int num_igt_formats;
+	int i;
+
+	igt_get_all_cairo_formats(&igt_formats, &num_igt_formats);
+	for (i = 0; i < num_igt_formats; i++) {
+		if (pixel_format == igt_formats[i])
+			return true;
+	}
+	return false;
+}
+
+int igt_get_format_count_plane(igt_plane_t *plane)
+{
+	if (plane)
+		return plane->drm_plane->count_formats;
+
+	return -EINVAL;
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 2a480bf..ced7d73 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -389,6 +389,10 @@  void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
 void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
 void igt_wait_for_vblank_count(int drm_fd, enum pipe pipe, int count);
 
+bool igt_is_cairo_supported_format(uint32_t pixel_format);
+int igt_get_format_count_plane(igt_plane_t *plane);
+const char *igt_get_format_name(uint32_t format);
+
 static inline bool igt_output_is_connected(igt_output_t *output)
 {
 	/* Something went wrong during probe? */
@@ -486,6 +490,9 @@  static inline bool igt_output_is_connected(igt_output_t *output)
 	for (int j__ = 0; assert(igt_can_fail()), (plane) = &(display)->pipes[(pipe)].planes[j__], \
 		     j__ < (display)->pipes[(pipe)].n_planes; j__++)
 
+#define for_each_format_in_plane(plane, format)	\
+	for (int k__ = 0; (format) = plane->drm_plane->formats[k__], k__ < plane->drm_plane->count_formats; k__++)
+
 #define IGT_FIXED(i,f)	((i) << 16 | (f))
 
 /**