[RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions

[RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions

[Carlos Eduardo Gallo Filho]

There's duplicated functions on drm that do the same job of calculating
the size of planes from a drm_format_info and the size of its first
plane. So this patchset throw away the more specific version intended
to be used from a given framebuffer and make the generic version way
more portable against the drivers.

Thanks,
Carlos

---
v2:
  - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement
    for core helpers".

Carlos Eduardo Gallo Filho (2):
  drm: Remove plane hsub/vsub alignment requirement for core helpers
  drm: Replace drm_framebuffer plane size functions with its equivalents

 drivers/gpu/drm/drm_framebuffer.c       | 64 ++-----------------------
 drivers/gpu/drm/i915/display/intel_fb.c |  2 +-
 include/drm/drm_fourcc.h                |  5 +-
 include/drm/drm_framebuffer.h           |  5 --
 4 files changed, 8 insertions(+), 68 deletions(-)

-- 
2.41.0

[Intel-gfx] [RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions

[Carlos Eduardo Gallo Filho]

There's duplicated functions on drm that do the same job of calculating
the size of planes from a drm_format_info and the size of its first
plane. So this patchset throw away the more specific version intended
to be used from a given framebuffer and make the generic version way
more portable against the drivers.

Thanks,
Carlos

---
v2:
  - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement
    for core helpers".

Carlos Eduardo Gallo Filho (2):
  drm: Remove plane hsub/vsub alignment requirement for core helpers
  drm: Replace drm_framebuffer plane size functions with its equivalents

 drivers/gpu/drm/drm_framebuffer.c       | 64 ++-----------------------
 drivers/gpu/drm/i915/display/intel_fb.c |  2 +-
 include/drm/drm_fourcc.h                |  5 +-
 include/drm/drm_framebuffer.h           |  5 --
 4 files changed, 8 insertions(+), 68 deletions(-)

-- 
2.41.0

[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers

[Carlos Eduardo Gallo Filho]

The drm_format_info_plane_{height,width} functions was implemented using
regular division for the plane size calculation, which cause issues [1][2]
when used on contexts where the dimensions are misaligned with relation
to the subsampling factors. So, replace the regular division by the
DIV_ROUND_UP macro.

This allows these functions to be used in more drivers, making further
work to bring more core presence on them possible.

[1] http://patchwork.freedesktop.org/patch/msgid/20170321181218.10042-3-ville.syrjala@linux.intel.com
[2] https://patchwork.freedesktop.org/patch/msgid/20211026225105.2783797-2-imre.deak@intel.com

Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
---
 include/drm/drm_fourcc.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 532ae78ca747..ccf91daa4307 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -22,6 +22,7 @@
 #ifndef __DRM_FOURCC_H__
 #define __DRM_FOURCC_H__
 
+#include <linux/math.h>
 #include <linux/types.h>
 #include <uapi/drm/drm_fourcc.h>
 
@@ -279,7 +280,7 @@ int drm_format_info_plane_width(const struct drm_format_info *info, int width,
 	if (plane == 0)
 		return width;
 
-	return width / info->hsub;
+	return DIV_ROUND_UP(width, info->hsub);
 }
 
 /**
@@ -301,7 +302,7 @@ int drm_format_info_plane_height(const struct drm_format_info *info, int height,
 	if (plane == 0)
 		return height;
 
-	return height / info->vsub;
+	return DIV_ROUND_UP(height, info->vsub);
 }
 
 const struct drm_format_info *__drm_format_info(u32 format);
-- 
2.41.0

[Intel-gfx] [PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers

[Carlos Eduardo Gallo Filho]

The drm_format_info_plane_{height,width} functions was implemented using
regular division for the plane size calculation, which cause issues [1][2]
when used on contexts where the dimensions are misaligned with relation
to the subsampling factors. So, replace the regular division by the
DIV_ROUND_UP macro.

This allows these functions to be used in more drivers, making further
work to bring more core presence on them possible.

[1] http://patchwork.freedesktop.org/patch/msgid/20170321181218.10042-3-ville.syrjala@linux.intel.com
[2] https://patchwork.freedesktop.org/patch/msgid/20211026225105.2783797-2-imre.deak@intel.com

Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
---
 include/drm/drm_fourcc.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 532ae78ca747..ccf91daa4307 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -22,6 +22,7 @@
 #ifndef __DRM_FOURCC_H__
 #define __DRM_FOURCC_H__
 
+#include <linux/math.h>
 #include <linux/types.h>
 #include <uapi/drm/drm_fourcc.h>
 
@@ -279,7 +280,7 @@ int drm_format_info_plane_width(const struct drm_format_info *info, int width,
 	if (plane == 0)
 		return width;
 
-	return width / info->hsub;
+	return DIV_ROUND_UP(width, info->hsub);
 }
 
 /**
@@ -301,7 +302,7 @@ int drm_format_info_plane_height(const struct drm_format_info *info, int height,
 	if (plane == 0)
 		return height;
 
-	return height / info->vsub;
+	return DIV_ROUND_UP(height, info->vsub);
 }
 
 const struct drm_format_info *__drm_format_info(u32 format);
-- 
2.41.0

[PATCH v2 2/2] drm: Replace drm_framebuffer plane size functions with its equivalents

[Carlos Eduardo Gallo Filho]

The functions drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} do exactly the same job of its
equivalents drm_format_info_plane_{width,height} from drm_fourcc.

The only reason to have these functions on drm_framebuffer
would be if they would added a abstraction layer to call it just
passing a drm_framebuffer pointer and the desired plane index,
which is not the case, where these functions actually implements
just part of it. In the actual implementation, every call to both
drm_framebuffer_plane_{width,height} and fb_plane_{width,height} should
pass some drm_framebuffer attribute, which is the same as calling the
drm_format_info_plane_{width,height} functions.

The drm_format_info_pane_{width,height} functions are much more
consistent in both its implementation and its location on code. The
kind of calculation that they do is intrinsically derivated from the
drm_format_info struct and has not to do with drm_framebuffer, except
by the potential motivation described above, which is still not a good
justification to have drm_framebuffer functions to calculate it.

So, replace each drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} call to drm_format_info_plane_{width,height}
and remove them.

Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
---
 drivers/gpu/drm/drm_framebuffer.c       | 64 ++-----------------------
 drivers/gpu/drm/i915/display/intel_fb.c |  2 +-
 include/drm/drm_framebuffer.h           |  5 --
 3 files changed, 5 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index ba51deb6d042..d3ba0698b84b 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -151,24 +151,6 @@ int drm_mode_addfb_ioctl(struct drm_device *dev,
 	return drm_mode_addfb(dev, data, file_priv);
 }
 
-static int fb_plane_width(int width,
-			  const struct drm_format_info *format, int plane)
-{
-	if (plane == 0)
-		return width;
-
-	return DIV_ROUND_UP(width, format->hsub);
-}
-
-static int fb_plane_height(int height,
-			   const struct drm_format_info *format, int plane)
-{
-	if (plane == 0)
-		return height;
-
-	return DIV_ROUND_UP(height, format->vsub);
-}
-
 static int framebuffer_check(struct drm_device *dev,
 			     const struct drm_mode_fb_cmd2 *r)
 {
@@ -196,8 +178,8 @@ static int framebuffer_check(struct drm_device *dev,
 	info = drm_get_format_info(dev, r);
 
 	for (i = 0; i < info->num_planes; i++) {
-		unsigned int width = fb_plane_width(r->width, info, i);
-		unsigned int height = fb_plane_height(r->height, info, i);
+		unsigned int width = drm_format_info_plane_width(info, r->width, i);
+		unsigned int height = drm_format_info_plane_height(info, r->height, i);
 		unsigned int block_size = info->char_per_block[i];
 		u64 min_pitch = drm_format_info_min_pitch(info, i, width);
 
@@ -1136,44 +1118,6 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
 }
 EXPORT_SYMBOL(drm_framebuffer_remove);
 
-/**
- * drm_framebuffer_plane_width - width of the plane given the first plane
- * @width: width of the first plane
- * @fb: the framebuffer
- * @plane: plane index
- *
- * Returns:
- * The width of @plane, given that the width of the first plane is @width.
- */
-int drm_framebuffer_plane_width(int width,
-				const struct drm_framebuffer *fb, int plane)
-{
-	if (plane >= fb->format->num_planes)
-		return 0;
-
-	return fb_plane_width(width, fb->format, plane);
-}
-EXPORT_SYMBOL(drm_framebuffer_plane_width);
-
-/**
- * drm_framebuffer_plane_height - height of the plane given the first plane
- * @height: height of the first plane
- * @fb: the framebuffer
- * @plane: plane index
- *
- * Returns:
- * The height of @plane, given that the height of the first plane is @height.
- */
-int drm_framebuffer_plane_height(int height,
-				 const struct drm_framebuffer *fb, int plane)
-{
-	if (plane >= fb->format->num_planes)
-		return 0;
-
-	return fb_plane_height(height, fb->format, plane);
-}
-EXPORT_SYMBOL(drm_framebuffer_plane_height);
-
 void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
 				const struct drm_framebuffer *fb)
 {
@@ -1189,8 +1133,8 @@ void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
 
 	for (i = 0; i < fb->format->num_planes; i++) {
 		drm_printf_indent(p, indent + 1, "size[%u]=%dx%d\n", i,
-				  drm_framebuffer_plane_width(fb->width, fb, i),
-				  drm_framebuffer_plane_height(fb->height, fb, i));
+				  drm_format_info_plane_width(fb->format, fb->width, i),
+				  drm_format_info_plane_height(fb->format, fb->height, i));
 		drm_printf_indent(p, indent + 1, "pitch[%u]=%u\n", i, fb->pitches[i]);
 		drm_printf_indent(p, indent + 1, "offset[%u]=%u\n", i, fb->offsets[i]);
 		drm_printf_indent(p, indent + 1, "obj[%u]:%s\n", i,
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
index 446bbf7986b6..5de2453ff7a3 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -1113,7 +1113,7 @@ static int intel_fb_offset_to_xy(int *x, int *y,
 		return -EINVAL;
 	}
 
-	height = drm_framebuffer_plane_height(fb->height, fb, color_plane);
+	height = drm_format_info_plane_height(fb->format, fb->height, color_plane);
 	height = ALIGN(height, intel_tile_height(fb, color_plane));
 
 	/* Catch potential overflows early */
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index 0dcc07b68654..80ece7b6dd9b 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -292,11 +292,6 @@ static inline void drm_framebuffer_assign(struct drm_framebuffer **p,
 	     &fb->head != (&(dev)->mode_config.fb_list);			\
 	     fb = list_next_entry(fb, head))
 
-int drm_framebuffer_plane_width(int width,
-				const struct drm_framebuffer *fb, int plane);
-int drm_framebuffer_plane_height(int height,
-				 const struct drm_framebuffer *fb, int plane);
-
 /**
  * struct drm_afbc_framebuffer - a special afbc frame buffer object
  *
-- 
2.41.0

[Intel-gfx] [PATCH v2 2/2] drm: Replace drm_framebuffer plane size functions with its equivalents

[Carlos Eduardo Gallo Filho]

The functions drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} do exactly the same job of its
equivalents drm_format_info_plane_{width,height} from drm_fourcc.

The only reason to have these functions on drm_framebuffer
would be if they would added a abstraction layer to call it just
passing a drm_framebuffer pointer and the desired plane index,
which is not the case, where these functions actually implements
just part of it. In the actual implementation, every call to both
drm_framebuffer_plane_{width,height} and fb_plane_{width,height} should
pass some drm_framebuffer attribute, which is the same as calling the
drm_format_info_plane_{width,height} functions.

The drm_format_info_pane_{width,height} functions are much more
consistent in both its implementation and its location on code. The
kind of calculation that they do is intrinsically derivated from the
drm_format_info struct and has not to do with drm_framebuffer, except
by the potential motivation described above, which is still not a good
justification to have drm_framebuffer functions to calculate it.

So, replace each drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} call to drm_format_info_plane_{width,height}
and remove them.

Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
---
 drivers/gpu/drm/drm_framebuffer.c       | 64 ++-----------------------
 drivers/gpu/drm/i915/display/intel_fb.c |  2 +-
 include/drm/drm_framebuffer.h           |  5 --
 3 files changed, 5 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index ba51deb6d042..d3ba0698b84b 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -151,24 +151,6 @@ int drm_mode_addfb_ioctl(struct drm_device *dev,
 	return drm_mode_addfb(dev, data, file_priv);
 }
 
-static int fb_plane_width(int width,
-			  const struct drm_format_info *format, int plane)
-{
-	if (plane == 0)
-		return width;
-
-	return DIV_ROUND_UP(width, format->hsub);
-}
-
-static int fb_plane_height(int height,
-			   const struct drm_format_info *format, int plane)
-{
-	if (plane == 0)
-		return height;
-
-	return DIV_ROUND_UP(height, format->vsub);
-}
-
 static int framebuffer_check(struct drm_device *dev,
 			     const struct drm_mode_fb_cmd2 *r)
 {
@@ -196,8 +178,8 @@ static int framebuffer_check(struct drm_device *dev,
 	info = drm_get_format_info(dev, r);
 
 	for (i = 0; i < info->num_planes; i++) {
-		unsigned int width = fb_plane_width(r->width, info, i);
-		unsigned int height = fb_plane_height(r->height, info, i);
+		unsigned int width = drm_format_info_plane_width(info, r->width, i);
+		unsigned int height = drm_format_info_plane_height(info, r->height, i);
 		unsigned int block_size = info->char_per_block[i];
 		u64 min_pitch = drm_format_info_min_pitch(info, i, width);
 
@@ -1136,44 +1118,6 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
 }
 EXPORT_SYMBOL(drm_framebuffer_remove);
 
-/**
- * drm_framebuffer_plane_width - width of the plane given the first plane
- * @width: width of the first plane
- * @fb: the framebuffer
- * @plane: plane index
- *
- * Returns:
- * The width of @plane, given that the width of the first plane is @width.
- */
-int drm_framebuffer_plane_width(int width,
-				const struct drm_framebuffer *fb, int plane)
-{
-	if (plane >= fb->format->num_planes)
-		return 0;
-
-	return fb_plane_width(width, fb->format, plane);
-}
-EXPORT_SYMBOL(drm_framebuffer_plane_width);
-
-/**
- * drm_framebuffer_plane_height - height of the plane given the first plane
- * @height: height of the first plane
- * @fb: the framebuffer
- * @plane: plane index
- *
- * Returns:
- * The height of @plane, given that the height of the first plane is @height.
- */
-int drm_framebuffer_plane_height(int height,
-				 const struct drm_framebuffer *fb, int plane)
-{
-	if (plane >= fb->format->num_planes)
-		return 0;
-
-	return fb_plane_height(height, fb->format, plane);
-}
-EXPORT_SYMBOL(drm_framebuffer_plane_height);
-
 void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
 				const struct drm_framebuffer *fb)
 {
@@ -1189,8 +1133,8 @@ void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
 
 	for (i = 0; i < fb->format->num_planes; i++) {
 		drm_printf_indent(p, indent + 1, "size[%u]=%dx%d\n", i,
-				  drm_framebuffer_plane_width(fb->width, fb, i),
-				  drm_framebuffer_plane_height(fb->height, fb, i));
+				  drm_format_info_plane_width(fb->format, fb->width, i),
+				  drm_format_info_plane_height(fb->format, fb->height, i));
 		drm_printf_indent(p, indent + 1, "pitch[%u]=%u\n", i, fb->pitches[i]);
 		drm_printf_indent(p, indent + 1, "offset[%u]=%u\n", i, fb->offsets[i]);
 		drm_printf_indent(p, indent + 1, "obj[%u]:%s\n", i,
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
index 446bbf7986b6..5de2453ff7a3 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -1113,7 +1113,7 @@ static int intel_fb_offset_to_xy(int *x, int *y,
 		return -EINVAL;
 	}
 
-	height = drm_framebuffer_plane_height(fb->height, fb, color_plane);
+	height = drm_format_info_plane_height(fb->format, fb->height, color_plane);
 	height = ALIGN(height, intel_tile_height(fb, color_plane));
 
 	/* Catch potential overflows early */
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index 0dcc07b68654..80ece7b6dd9b 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -292,11 +292,6 @@ static inline void drm_framebuffer_assign(struct drm_framebuffer **p,
 	     &fb->head != (&(dev)->mode_config.fb_list);			\
 	     fb = list_next_entry(fb, head))
 
-int drm_framebuffer_plane_width(int width,
-				const struct drm_framebuffer *fb, int plane);
-int drm_framebuffer_plane_height(int height,
-				 const struct drm_framebuffer *fb, int plane);
-
 /**
  * struct drm_afbc_framebuffer - a special afbc frame buffer object
  *
-- 
2.41.0

Re: [RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions

[Helen Koike]

On 26/09/2023 11:15, Carlos Eduardo Gallo Filho wrote:
> There's duplicated functions on drm that do the same job of calculating
> the size of planes from a drm_format_info and the size of its first
> plane. So this patchset throw away the more specific version intended
> to be used from a given framebuffer and make the generic version way
> more portable against the drivers.
> 
> Thanks,
> Carlos

Hey, thanks for your patch.

Do you mind testing on drm/ci and sending here the link of the pipeline 
with the test? It would be awesome to get your feedback on the CI

See instructions on Documentation/gpu/automated_testing.rst

In short you just need an account on gitlab.freedesktop.org, access
https://gitlab.freedesktop.org/janedoe/linux/-/settings/ci_cd), change 
the CI/CD configuration file from .gitlab-ci.yml to 
drivers/gpu/drm/ci/gitlab-ci.yml, now you can execute tests going to 
pipelines (the first one you need to create a new pipeline by hand).

Let me know if you have any questions, I'm koike on irc.

Thanks
Helen

> 
> ---
> v2:
>    - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement
>      for core helpers".
> 
> Carlos Eduardo Gallo Filho (2):
>    drm: Remove plane hsub/vsub alignment requirement for core helpers
>    drm: Replace drm_framebuffer plane size functions with its equivalents
> 
>   drivers/gpu/drm/drm_framebuffer.c       | 64 ++-----------------------
>   drivers/gpu/drm/i915/display/intel_fb.c |  2 +-
>   include/drm/drm_fourcc.h                |  5 +-
>   include/drm/drm_framebuffer.h           |  5 --
>   4 files changed, 8 insertions(+), 68 deletions(-)
> 

Re: [Intel-gfx] [RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions

[Helen Koike]

On 26/09/2023 11:15, Carlos Eduardo Gallo Filho wrote:
> There's duplicated functions on drm that do the same job of calculating
> the size of planes from a drm_format_info and the size of its first
> plane. So this patchset throw away the more specific version intended
> to be used from a given framebuffer and make the generic version way
> more portable against the drivers.
> 
> Thanks,
> Carlos

Hey, thanks for your patch.

Do you mind testing on drm/ci and sending here the link of the pipeline 
with the test? It would be awesome to get your feedback on the CI

See instructions on Documentation/gpu/automated_testing.rst

In short you just need an account on gitlab.freedesktop.org, access
https://gitlab.freedesktop.org/janedoe/linux/-/settings/ci_cd), change 
the CI/CD configuration file from .gitlab-ci.yml to 
drivers/gpu/drm/ci/gitlab-ci.yml, now you can execute tests going to 
pipelines (the first one you need to create a new pipeline by hand).

Let me know if you have any questions, I'm koike on irc.

Thanks
Helen

> 
> ---
> v2:
>    - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement
>      for core helpers".
> 
> Carlos Eduardo Gallo Filho (2):
>    drm: Remove plane hsub/vsub alignment requirement for core helpers
>    drm: Replace drm_framebuffer plane size functions with its equivalents
> 
>   drivers/gpu/drm/drm_framebuffer.c       | 64 ++-----------------------
>   drivers/gpu/drm/i915/display/intel_fb.c |  2 +-
>   include/drm/drm_fourcc.h                |  5 +-
>   include/drm/drm_framebuffer.h           |  5 --
>   4 files changed, 8 insertions(+), 68 deletions(-)
> 

Re: [RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1362 bytes --]

Hi

Am 26.09.23 um 16:15 schrieb Carlos Eduardo Gallo Filho:
> There's duplicated functions on drm that do the same job of calculating
> the size of planes from a drm_format_info and the size of its first
> plane. So this patchset throw away the more specific version intended
> to be used from a given framebuffer and make the generic version way
> more portable against the drivers.

For both patches

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regards
Thomas

> 
> Thanks,
> Carlos
> 
> ---
> v2:
>    - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement
>      for core helpers".
> 
> Carlos Eduardo Gallo Filho (2):
>    drm: Remove plane hsub/vsub alignment requirement for core helpers
>    drm: Replace drm_framebuffer plane size functions with its equivalents
> 
>   drivers/gpu/drm/drm_framebuffer.c       | 64 ++-----------------------
>   drivers/gpu/drm/i915/display/intel_fb.c |  2 +-
>   include/drm/drm_fourcc.h                |  5 +-
>   include/drm/drm_framebuffer.h           |  5 --
>   4 files changed, 8 insertions(+), 68 deletions(-)
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [Intel-gfx] [RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1362 bytes --]

Hi

Am 26.09.23 um 16:15 schrieb Carlos Eduardo Gallo Filho:
> There's duplicated functions on drm that do the same job of calculating
> the size of planes from a drm_format_info and the size of its first
> plane. So this patchset throw away the more specific version intended
> to be used from a given framebuffer and make the generic version way
> more portable against the drivers.

For both patches

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regards
Thomas

> 
> Thanks,
> Carlos
> 
> ---
> v2:
>    - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement
>      for core helpers".
> 
> Carlos Eduardo Gallo Filho (2):
>    drm: Remove plane hsub/vsub alignment requirement for core helpers
>    drm: Replace drm_framebuffer plane size functions with its equivalents
> 
>   drivers/gpu/drm/drm_framebuffer.c       | 64 ++-----------------------
>   drivers/gpu/drm/i915/display/intel_fb.c |  2 +-
>   include/drm/drm_fourcc.h                |  5 +-
>   include/drm/drm_framebuffer.h           |  5 --
>   4 files changed, 8 insertions(+), 68 deletions(-)
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Refactor plane size calculation by core helper functions (rev2)

[Patchwork]

== Series Details ==

Series: drm: Refactor plane size calculation by core helper functions (rev2)
URL   : https://patchwork.freedesktop.org/series/121012/
State : warning

== Summary ==

Error: dim checkpatch failed
3df72ee1d291 drm: Remove plane hsub/vsub alignment requirement for core helpers
-:16: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#16: 
[1] http://patchwork.freedesktop.org/patch/msgid/20170321181218.10042-3-ville.syrjala@linux.intel.com

total: 0 errors, 1 warnings, 0 checks, 23 lines checked
ebc0690a489b drm: Replace drm_framebuffer plane size functions with its equivalents

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: Refactor plane size calculation by core helper functions (rev2)

[Patchwork]

== Series Details ==

Series: drm: Refactor plane size calculation by core helper functions (rev2)
URL   : https://patchwork.freedesktop.org/series/121012/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm: Refactor plane size calculation by core helper functions (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 14321 bytes --]

== Series Details ==

Series: drm: Refactor plane size calculation by core helper functions (rev2)
URL   : https://patchwork.freedesktop.org/series/121012/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13682 -> Patchwork_121012v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_121012v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_121012v2, please notify your bug team (lgci.bug.filing@intel.com) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/index.html

Participating hosts (41 -> 42)
------------------------------

  Additional (2): fi-kbl-soraka bat-dg2-8 
  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_121012v2:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-1:
    - fi-elk-e7500:       [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/fi-elk-e7500/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/fi-elk-e7500/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-1.html

  
Known issues
------------

  Here are the changes found in Patchwork_121012v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s0@lmem0:
    - bat-dg2-9:          [PASS][3] -> [INCOMPLETE][4] ([i915#9275])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#2190])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#4613]) +3 other tests skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@gem_mmap@basic:
    - bat-dg2-8:          NOTRUN -> [SKIP][7] ([i915#4083])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@gem_mmap@basic.html

  * igt@gem_render_tiled_blits@basic:
    - bat-dg2-8:          NOTRUN -> [SKIP][8] ([i915#4079]) +1 other test skip
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@gem_render_tiled_blits@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-dg2-8:          NOTRUN -> [SKIP][9] ([i915#4077]) +2 other tests skip
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@gem_tiled_fence_blits@basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg2-8:          NOTRUN -> [SKIP][10] ([i915#6621])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-apl-guc:         [PASS][11] -> [DMESG-FAIL][12] ([i915#5334])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][13] ([i915#1886] / [i915#7913])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-dg2-8:          NOTRUN -> [SKIP][14] ([i915#6645])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy:
    - bat-dg2-8:          NOTRUN -> [SKIP][15] ([i915#4212]) +6 other tests skip
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-dg2-8:          NOTRUN -> [SKIP][16] ([i915#5190])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
    - bat-dg2-8:          NOTRUN -> [SKIP][17] ([i915#4215] / [i915#5190])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@kms_addfb_basic@basic-y-tiled-legacy.html

  * igt@kms_addfb_basic@tile-pitch-mismatch:
    - bat-dg2-8:          NOTRUN -> [SKIP][18] ([i915#4212] / [i915#5608])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@kms_addfb_basic@tile-pitch-mismatch.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][19] ([fdo#109271]) +9 other tests skip
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/fi-kbl-soraka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-dg2-8:          NOTRUN -> [SKIP][20] ([i915#4103] / [i915#4213] / [i915#5608]) +1 other test skip
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-dg2-8:          NOTRUN -> [SKIP][21] ([fdo#109285])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-dg2-8:          NOTRUN -> [SKIP][22] ([i915#5274])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence:
    - bat-adlp-9:         NOTRUN -> [SKIP][23] ([i915#3546]) +2 other tests skip
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-adlp-9/igt@kms_pipe_crc_basic@read-crc-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-c-dp-5:
    - bat-adlp-11:        [PASS][24] -> [ABORT][25] ([i915#8668])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-adlp-11/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-c-dp-5.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-adlp-11/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-c-dp-5.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
    - bat-rplp-1:         [PASS][26] -> [ABORT][27] ([i915#8668])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html

  * igt@kms_psr@cursor_plane_move:
    - bat-dg2-8:          NOTRUN -> [SKIP][28] ([i915#1072]) +3 other tests skip
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@kms_psr@cursor_plane_move.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg2-8:          NOTRUN -> [SKIP][29] ([i915#3555])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-dg2-8:          NOTRUN -> [SKIP][30] ([i915#3708])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-gtt:
    - bat-dg2-8:          NOTRUN -> [SKIP][31] ([i915#3708] / [i915#4077]) +1 other test skip
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@prime_vgem@basic-gtt.html

  * igt@prime_vgem@basic-write:
    - bat-dg2-8:          NOTRUN -> [SKIP][32] ([i915#3291] / [i915#3708]) +2 other tests skip
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-8/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@kms_chamelium_frames@dp-crc-fast:
    - {bat-dg2-13}:       [DMESG-WARN][33] ([Intel XE#485]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-dg2-13/igt@kms_chamelium_frames@dp-crc-fast.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-dg2-13/igt@kms_chamelium_frames@dp-crc-fast.html

  * igt@kms_flip@basic-flip-vs-modeset@c-dp6:
    - bat-adlp-11:        [FAIL][35] ([i915#6121]) -> [PASS][36] +8 other tests pass
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@c-dp6.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@c-dp6.html

  * igt@kms_flip@basic-plain-flip@b-dp6:
    - bat-adlp-11:        [DMESG-WARN][37] ([i915#6868]) -> [PASS][38] +1 other test pass
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-adlp-11/igt@kms_flip@basic-plain-flip@b-dp6.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-adlp-11/igt@kms_flip@basic-plain-flip@b-dp6.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-5:
    - bat-adlp-11:        [DMESG-FAIL][39] ([i915#6868]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-5.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-5.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-d-dp-5:
    - bat-adlp-11:        [FAIL][41] ([i915#9047]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-d-dp-5.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-d-dp-5.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-b-dp-6:
    - bat-adlp-11:        [ABORT][43] ([i915#8668]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-adlp-11/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-b-dp-6.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-adlp-11/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-b-dp-6.html

  
#### Warnings ####

  * igt@kms_force_connector_basic@force-edid:
    - bat-adlp-11:        [FAIL][45] ([i915#8803]) -> [SKIP][46] ([i915#4093])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13682/bat-adlp-11/igt@kms_force_connector_basic@force-edid.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/bat-adlp-11/igt@kms_force_connector_basic@force-edid.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [Intel XE#485]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/485
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4093]: https://gitlab.freedesktop.org/drm/intel/issues/4093
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5608]: https://gitlab.freedesktop.org/drm/intel/issues/5608
  [i915#6121]: https://gitlab.freedesktop.org/drm/intel/issues/6121
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#6868]: https://gitlab.freedesktop.org/drm/intel/issues/6868
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
  [i915#8803]: https://gitlab.freedesktop.org/drm/intel/issues/8803
  [i915#9047]: https://gitlab.freedesktop.org/drm/intel/issues/9047
  [i915#9275]: https://gitlab.freedesktop.org/drm/intel/issues/9275


Build changes
-------------

  * Linux: CI_DRM_13682 -> Patchwork_121012v2

  CI-20190529: 20190529
  CI_DRM_13682: a42554bf0755b80fdfb8e91ca35ae6835bb3534d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7503: 7503
  Patchwork_121012v2: a42554bf0755b80fdfb8e91ca35ae6835bb3534d @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

d99bba12fc52 drm: Replace drm_framebuffer plane size functions with its equivalents
6a36672d7a4a drm: Remove plane hsub/vsub alignment requirement for core helpers

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121012v2/index.html

[-- Attachment #2: Type: text/html, Size: 16466 bytes --]

Re: [PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers

[André Almeida]

Hi Carlos,

On 9/26/23 16:15, Carlos Eduardo Gallo Filho wrote:
> The drm_format_info_plane_{height,width} functions was implemented using
> regular division for the plane size calculation, which cause issues [1][2]
> when used on contexts where the dimensions are misaligned with relation
> to the subsampling factors. So, replace the regular division by the
> DIV_ROUND_UP macro.
>
> This allows these functions to be used in more drivers, making further
> work to bring more core presence on them possible.
>
> [1] http://patchwork.freedesktop.org/patch/msgid/20170321181218.10042-3-ville.syrjala@linux.intel.com
> [2] https://patchwork.freedesktop.org/patch/msgid/20211026225105.2783797-2-imre.deak@intel.com

Prefer to use lore:

https://lore.kernel.org/dri-devel/20170321181218.10042-3-ville.syrjala@linux.intel.com/

https://lore.kernel.org/intel-gfx/20211026225105.2783797-2-imre.deak@intel.com/


Other than that,

Reviewed-by: André Almeida <andrealmeid@igalia.com>

>
> Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
> ---
>   include/drm/drm_fourcc.h | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 532ae78ca747..ccf91daa4307 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -22,6 +22,7 @@
>   #ifndef __DRM_FOURCC_H__
>   #define __DRM_FOURCC_H__
>   
> +#include <linux/math.h>
>   #include <linux/types.h>
>   #include <uapi/drm/drm_fourcc.h>
>   
> @@ -279,7 +280,7 @@ int drm_format_info_plane_width(const struct drm_format_info *info, int width,
>   	if (plane == 0)
>   		return width;
>   
> -	return width / info->hsub;
> +	return DIV_ROUND_UP(width, info->hsub);
>   }
>   
>   /**
> @@ -301,7 +302,7 @@ int drm_format_info_plane_height(const struct drm_format_info *info, int height,
>   	if (plane == 0)
>   		return height;
>   
> -	return height / info->vsub;
> +	return DIV_ROUND_UP(height, info->vsub);
>   }
>   
>   const struct drm_format_info *__drm_format_info(u32 format);

Re: [Intel-gfx] [PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers

[André Almeida]

Hi Carlos,

On 9/26/23 16:15, Carlos Eduardo Gallo Filho wrote:
> The drm_format_info_plane_{height,width} functions was implemented using
> regular division for the plane size calculation, which cause issues [1][2]
> when used on contexts where the dimensions are misaligned with relation
> to the subsampling factors. So, replace the regular division by the
> DIV_ROUND_UP macro.
>
> This allows these functions to be used in more drivers, making further
> work to bring more core presence on them possible.
>
> [1] http://patchwork.freedesktop.org/patch/msgid/20170321181218.10042-3-ville.syrjala@linux.intel.com
> [2] https://patchwork.freedesktop.org/patch/msgid/20211026225105.2783797-2-imre.deak@intel.com

Prefer to use lore:

https://lore.kernel.org/dri-devel/20170321181218.10042-3-ville.syrjala@linux.intel.com/

https://lore.kernel.org/intel-gfx/20211026225105.2783797-2-imre.deak@intel.com/


Other than that,

Reviewed-by: André Almeida <andrealmeid@igalia.com>

>
> Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
> ---
>   include/drm/drm_fourcc.h | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 532ae78ca747..ccf91daa4307 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -22,6 +22,7 @@
>   #ifndef __DRM_FOURCC_H__
>   #define __DRM_FOURCC_H__
>   
> +#include <linux/math.h>
>   #include <linux/types.h>
>   #include <uapi/drm/drm_fourcc.h>
>   
> @@ -279,7 +280,7 @@ int drm_format_info_plane_width(const struct drm_format_info *info, int width,
>   	if (plane == 0)
>   		return width;
>   
> -	return width / info->hsub;
> +	return DIV_ROUND_UP(width, info->hsub);
>   }
>   
>   /**
> @@ -301,7 +302,7 @@ int drm_format_info_plane_height(const struct drm_format_info *info, int height,
>   	if (plane == 0)
>   		return height;
>   
> -	return height / info->vsub;
> +	return DIV_ROUND_UP(height, info->vsub);
>   }
>   
>   const struct drm_format_info *__drm_format_info(u32 format);

Re: [PATCH v2 2/2] drm: Replace drm_framebuffer plane size functions with its equivalents

[André Almeida]

On 9/26/23 16:15, Carlos Eduardo Gallo Filho wrote:
> The functions drm_framebuffer_plane_{width,height} and
> fb_plane_{width,height} do exactly the same job of its
> equivalents drm_format_info_plane_{width,height} from drm_fourcc.
>
> The only reason to have these functions on drm_framebuffer
> would be if they would added a abstraction layer to call it just
> passing a drm_framebuffer pointer and the desired plane index,
> which is not the case, where these functions actually implements
> just part of it. In the actual implementation, every call to both
> drm_framebuffer_plane_{width,height} and fb_plane_{width,height} should
> pass some drm_framebuffer attribute, which is the same as calling the
> drm_format_info_plane_{width,height} functions.
>
> The drm_format_info_pane_{width,height} functions are much more
> consistent in both its implementation and its location on code. The
> kind of calculation that they do is intrinsically derivated from the
> drm_format_info struct and has not to do with drm_framebuffer, except
> by the potential motivation described above, which is still not a good
> justification to have drm_framebuffer functions to calculate it.
>
> So, replace each drm_framebuffer_plane_{width,height} and
> fb_plane_{width,height} call to drm_format_info_plane_{width,height}
> and remove them.
>
> Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
Reviewed-by: André Almeida <andrealmeid@igalia.com>

Re: [Intel-gfx] [PATCH v2 2/2] drm: Replace drm_framebuffer plane size functions with its equivalents

[André Almeida]

On 9/26/23 16:15, Carlos Eduardo Gallo Filho wrote:
> The functions drm_framebuffer_plane_{width,height} and
> fb_plane_{width,height} do exactly the same job of its
> equivalents drm_format_info_plane_{width,height} from drm_fourcc.
>
> The only reason to have these functions on drm_framebuffer
> would be if they would added a abstraction layer to call it just
> passing a drm_framebuffer pointer and the desired plane index,
> which is not the case, where these functions actually implements
> just part of it. In the actual implementation, every call to both
> drm_framebuffer_plane_{width,height} and fb_plane_{width,height} should
> pass some drm_framebuffer attribute, which is the same as calling the
> drm_format_info_plane_{width,height} functions.
>
> The drm_format_info_pane_{width,height} functions are much more
> consistent in both its implementation and its location on code. The
> kind of calculation that they do is intrinsically derivated from the
> drm_format_info struct and has not to do with drm_framebuffer, except
> by the potential motivation described above, which is still not a good
> justification to have drm_framebuffer functions to calculate it.
>
> So, replace each drm_framebuffer_plane_{width,height} and
> fb_plane_{width,height} call to drm_format_info_plane_{width,height}
> and remove them.
>
> Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
Reviewed-by: André Almeida <andrealmeid@igalia.com>