[PATCH 0/9] drm/simpledrm: Enable damage clips and virtual screens

[PATCH 0/9] drm/simpledrm: Enable damage clips and virtual screens

[Thomas Zimmermann]

Enable FB_DAMAGE_CLIPS with simpledrm for improved performance and/or
less overhead. With this in place, add support for virtual screens
(i.e., framebuffers that are larger than the display resolution). This
also enables fbdev panning and page flipping.

After the discussion and bug fixing wrt to fbdev overallocation, I
decided to add full support for this to simpledrm. Patches #1 to #5
change the format-helper functions accordingly. Destination buffers
are now clipped by the caller and all functions support a similar
feature set. This has some fallout in various drivers.

Patch #6 change fbdev emulation to support overallocation with
shadow buffers, even if the hardware buffer would be too small.

Patch #7 and #8 update simpledrm to enable damage clipping and virtual
screen sizes. Both feature go hand in hand, sort of. For shadow-
buffered planes, the DRM framebuffer lives in system memory. So the
maximum size of the virtual screen is somewhat arbitrary. We add two
constants for resonable maximum width and height of 4096 each.

Patch #9 adds documentation and a TODO item.

Tested with simpledrm. I also ran the recently posted fbdev panning
tests to make sure that the fbdev overallocation works correctly. [1]

[1] https://lists.freedesktop.org/archives/igt-dev/2021-October/036642.html

Thomas Zimmermann (9):
  drm/format-helper: Export drm_fb_clip_offset()
  drm/format-helper: Rework format-helper memcpy functions
  drm/format-helper: Add destination-buffer pitch to drm_fb_swab()
  drm/format-helper: Rework format-helper conversion functions
  drm/format-helper: Streamline blit-helper interface
  drm/fb-helper: Allocate shadow buffer of surface height
  drm/simpledrm: Enable FB_DAMAGE_CLIPS property
  drm/simpledrm: Support virtual screen sizes
  drm: Clarify semantics of struct
    drm_mode_config.{min,max}_{width,height}

 Documentation/gpu/todo.rst                  |  15 ++
 drivers/gpu/drm/drm_fb_helper.c             |   2 +-
 drivers/gpu/drm/drm_format_helper.c         | 236 ++++++++++----------
 drivers/gpu/drm/drm_mipi_dbi.c              |   6 +-
 drivers/gpu/drm/gud/gud_pipe.c              |  14 +-
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |   5 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c      |   4 +-
 drivers/gpu/drm/tiny/cirrus.c               |  24 +-
 drivers/gpu/drm/tiny/repaper.c              |   2 +-
 drivers/gpu/drm/tiny/simpledrm.c            |  41 +++-
 drivers/gpu/drm/tiny/st7586.c               |   2 +-
 include/drm/drm_format_helper.h             |  58 ++---
 include/drm/drm_gem_atomic_helper.h         |  18 ++
 include/drm/drm_mode_config.h               |  13 ++
 14 files changed, 254 insertions(+), 186 deletions(-)

--
2.33.0

[PATCH 0/9] drm/simpledrm: Enable damage clips and virtual screens

[Thomas Zimmermann]

Enable FB_DAMAGE_CLIPS with simpledrm for improved performance and/or
less overhead. With this in place, add support for virtual screens
(i.e., framebuffers that are larger than the display resolution). This
also enables fbdev panning and page flipping.

After the discussion and bug fixing wrt to fbdev overallocation, I
decided to add full support for this to simpledrm. Patches #1 to #5
change the format-helper functions accordingly. Destination buffers
are now clipped by the caller and all functions support a similar
feature set. This has some fallout in various drivers.

Patch #6 change fbdev emulation to support overallocation with
shadow buffers, even if the hardware buffer would be too small.

Patch #7 and #8 update simpledrm to enable damage clipping and virtual
screen sizes. Both feature go hand in hand, sort of. For shadow-
buffered planes, the DRM framebuffer lives in system memory. So the
maximum size of the virtual screen is somewhat arbitrary. We add two
constants for resonable maximum width and height of 4096 each.

Patch #9 adds documentation and a TODO item.

Tested with simpledrm. I also ran the recently posted fbdev panning
tests to make sure that the fbdev overallocation works correctly. [1]

[1] https://lists.freedesktop.org/archives/igt-dev/2021-October/036642.html

Thomas Zimmermann (9):
  drm/format-helper: Export drm_fb_clip_offset()
  drm/format-helper: Rework format-helper memcpy functions
  drm/format-helper: Add destination-buffer pitch to drm_fb_swab()
  drm/format-helper: Rework format-helper conversion functions
  drm/format-helper: Streamline blit-helper interface
  drm/fb-helper: Allocate shadow buffer of surface height
  drm/simpledrm: Enable FB_DAMAGE_CLIPS property
  drm/simpledrm: Support virtual screen sizes
  drm: Clarify semantics of struct
    drm_mode_config.{min,max}_{width,height}

 Documentation/gpu/todo.rst                  |  15 ++
 drivers/gpu/drm/drm_fb_helper.c             |   2 +-
 drivers/gpu/drm/drm_format_helper.c         | 236 ++++++++++----------
 drivers/gpu/drm/drm_mipi_dbi.c              |   6 +-
 drivers/gpu/drm/gud/gud_pipe.c              |  14 +-
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |   5 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c      |   4 +-
 drivers/gpu/drm/tiny/cirrus.c               |  24 +-
 drivers/gpu/drm/tiny/repaper.c              |   2 +-
 drivers/gpu/drm/tiny/simpledrm.c            |  41 +++-
 drivers/gpu/drm/tiny/st7586.c               |   2 +-
 include/drm/drm_format_helper.h             |  58 ++---
 include/drm/drm_gem_atomic_helper.h         |  18 ++
 include/drm/drm_mode_config.h               |  13 ++
 14 files changed, 254 insertions(+), 186 deletions(-)

--
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

[Thomas Zimmermann]

Provide a function that computes the offset into a blit destination
buffer. This will allow to move destination-buffer clipping into the
format-helper callers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 10 ++++++++--
 include/drm/drm_format_helper.h     |  4 ++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 69fde60e36b3..28e9d0d89270 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -17,12 +17,18 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_rect.h>
 
-static unsigned int clip_offset(struct drm_rect *clip,
-				unsigned int pitch, unsigned int cpp)
+static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
 {
 	return clip->y1 * pitch + clip->x1 * cpp;
 }
 
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
+				 const struct drm_rect *clip)
+{
+	return clip_offset(clip, pitch, format->cpp[0]);
+}
+EXPORT_SYMBOL(drm_fb_clip_offset);
+
 /**
  * drm_fb_memcpy - Copy clip buffer
  * @dst: Destination buffer
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index e86925cf07b9..90b9bd9ecb83 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -6,9 +6,13 @@
 #ifndef __LINUX_DRM_FORMAT_HELPER_H
 #define __LINUX_DRM_FORMAT_HELPER_H
 
+struct drm_format_info;
 struct drm_framebuffer;
 struct drm_rect;
 
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
+				 const struct drm_rect *clip);
+
 void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
 		   struct drm_rect *clip);
 void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
-- 
2.33.0

[PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

[Thomas Zimmermann]

Provide a function that computes the offset into a blit destination
buffer. This will allow to move destination-buffer clipping into the
format-helper callers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 10 ++++++++--
 include/drm/drm_format_helper.h     |  4 ++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 69fde60e36b3..28e9d0d89270 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -17,12 +17,18 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_rect.h>
 
-static unsigned int clip_offset(struct drm_rect *clip,
-				unsigned int pitch, unsigned int cpp)
+static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
 {
 	return clip->y1 * pitch + clip->x1 * cpp;
 }
 
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
+				 const struct drm_rect *clip)
+{
+	return clip_offset(clip, pitch, format->cpp[0]);
+}
+EXPORT_SYMBOL(drm_fb_clip_offset);
+
 /**
  * drm_fb_memcpy - Copy clip buffer
  * @dst: Destination buffer
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index e86925cf07b9..90b9bd9ecb83 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -6,9 +6,13 @@
 #ifndef __LINUX_DRM_FORMAT_HELPER_H
 #define __LINUX_DRM_FORMAT_HELPER_H
 
+struct drm_format_info;
 struct drm_framebuffer;
 struct drm_rect;
 
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
+				 const struct drm_rect *clip);
+
 void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
 		   struct drm_rect *clip);
 void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
-- 
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

[Sam Ravnborg]

Hi Thomas,

On Fri, Oct 22, 2021 at 03:28:21PM +0200, Thomas Zimmermann wrote:
> Provide a function that computes the offset into a blit destination
> buffer. This will allow to move destination-buffer clipping into the
> format-helper callers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_format_helper.c | 10 ++++++++--
>  include/drm/drm_format_helper.h     |  4 ++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 69fde60e36b3..28e9d0d89270 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -17,12 +17,18 @@
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_rect.h>
>  
> -static unsigned int clip_offset(struct drm_rect *clip,
> -				unsigned int pitch, unsigned int cpp)
> +static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
>  {
>  	return clip->y1 * pitch + clip->x1 * cpp;
>  }
>  
> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
> +				 const struct drm_rect *clip)
> +{
> +	return clip_offset(clip, pitch, format->cpp[0]);
> +}
> +EXPORT_SYMBOL(drm_fb_clip_offset);

Exported functions are expected to have kernel-doc documentation.
Just copy more or less from the changelog I think.

Anywhere else (I looked in struct drm_framebuffer) we only need unsigned
int for offsets and width/length - so I cannot see why we do an unsigned
int => unsigned long conversion here.

	Sam

> +
>  /**
>   * drm_fb_memcpy - Copy clip buffer
>   * @dst: Destination buffer
> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
> index e86925cf07b9..90b9bd9ecb83 100644
> --- a/include/drm/drm_format_helper.h
> +++ b/include/drm/drm_format_helper.h
> @@ -6,9 +6,13 @@
>  #ifndef __LINUX_DRM_FORMAT_HELPER_H
>  #define __LINUX_DRM_FORMAT_HELPER_H
>  
> +struct drm_format_info;
>  struct drm_framebuffer;
>  struct drm_rect;
>  
> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
> +				 const struct drm_rect *clip);
> +
>  void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
>  		   struct drm_rect *clip);
>  void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
> -- 
> 2.33.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

[Sam Ravnborg]

Hi Thomas,

On Fri, Oct 22, 2021 at 03:28:21PM +0200, Thomas Zimmermann wrote:
> Provide a function that computes the offset into a blit destination
> buffer. This will allow to move destination-buffer clipping into the
> format-helper callers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_format_helper.c | 10 ++++++++--
>  include/drm/drm_format_helper.h     |  4 ++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 69fde60e36b3..28e9d0d89270 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -17,12 +17,18 @@
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_rect.h>
>  
> -static unsigned int clip_offset(struct drm_rect *clip,
> -				unsigned int pitch, unsigned int cpp)
> +static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
>  {
>  	return clip->y1 * pitch + clip->x1 * cpp;
>  }
>  
> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
> +				 const struct drm_rect *clip)
> +{
> +	return clip_offset(clip, pitch, format->cpp[0]);
> +}
> +EXPORT_SYMBOL(drm_fb_clip_offset);

Exported functions are expected to have kernel-doc documentation.
Just copy more or less from the changelog I think.

Anywhere else (I looked in struct drm_framebuffer) we only need unsigned
int for offsets and width/length - so I cannot see why we do an unsigned
int => unsigned long conversion here.

	Sam

> +
>  /**
>   * drm_fb_memcpy - Copy clip buffer
>   * @dst: Destination buffer
> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
> index e86925cf07b9..90b9bd9ecb83 100644
> --- a/include/drm/drm_format_helper.h
> +++ b/include/drm/drm_format_helper.h
> @@ -6,9 +6,13 @@
>  #ifndef __LINUX_DRM_FORMAT_HELPER_H
>  #define __LINUX_DRM_FORMAT_HELPER_H
>  
> +struct drm_format_info;
>  struct drm_framebuffer;
>  struct drm_rect;
>  
> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
> +				 const struct drm_rect *clip);
> +
>  void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
>  		   struct drm_rect *clip);
>  void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
> -- 
> 2.33.0

Re: [PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

[Thomas Zimmermann]

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

Hi

Am 23.10.21 um 09:49 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Fri, Oct 22, 2021 at 03:28:21PM +0200, Thomas Zimmermann wrote:
>> Provide a function that computes the offset into a blit destination
>> buffer. This will allow to move destination-buffer clipping into the
>> format-helper callers.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/drm_format_helper.c | 10 ++++++++--
>>   include/drm/drm_format_helper.h     |  4 ++++
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
>> index 69fde60e36b3..28e9d0d89270 100644
>> --- a/drivers/gpu/drm/drm_format_helper.c
>> +++ b/drivers/gpu/drm/drm_format_helper.c
>> @@ -17,12 +17,18 @@
>>   #include <drm/drm_fourcc.h>
>>   #include <drm/drm_rect.h>
>>   
>> -static unsigned int clip_offset(struct drm_rect *clip,
>> -				unsigned int pitch, unsigned int cpp)
>> +static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
>>   {
>>   	return clip->y1 * pitch + clip->x1 * cpp;
>>   }
>>   
>> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
>> +				 const struct drm_rect *clip)
>> +{
>> +	return clip_offset(clip, pitch, format->cpp[0]);
>> +}
>> +EXPORT_SYMBOL(drm_fb_clip_offset);
> 
> Exported functions are expected to have kernel-doc documentation.
> Just copy more or less from the changelog I think.

That's an oversight. Sorry.

> 
> Anywhere else (I looked in struct drm_framebuffer) we only need unsigned
> int for offsets and width/length - so I cannot see why we do an unsigned
> int => unsigned long conversion here.

On ancient platforms, int was 16 bit wide. So for values that are array 
indices or buffer indices, I naturally use long, which is 32-bit at 
least. Never mind, it's not relevant any longer. I'll convert this code 
to unsigned int.

Best regards
Thomas

> 
> 	Sam
> 
>> +
>>   /**
>>    * drm_fb_memcpy - Copy clip buffer
>>    * @dst: Destination buffer
>> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
>> index e86925cf07b9..90b9bd9ecb83 100644
>> --- a/include/drm/drm_format_helper.h
>> +++ b/include/drm/drm_format_helper.h
>> @@ -6,9 +6,13 @@
>>   #ifndef __LINUX_DRM_FORMAT_HELPER_H
>>   #define __LINUX_DRM_FORMAT_HELPER_H
>>   
>> +struct drm_format_info;
>>   struct drm_framebuffer;
>>   struct drm_rect;
>>   
>> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
>> +				 const struct drm_rect *clip);
>> +
>>   void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
>>   		   struct drm_rect *clip);
>>   void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
>> -- 
>> 2.33.0

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

Re: [PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3094 bytes --]

Hi

Am 23.10.21 um 09:49 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Fri, Oct 22, 2021 at 03:28:21PM +0200, Thomas Zimmermann wrote:
>> Provide a function that computes the offset into a blit destination
>> buffer. This will allow to move destination-buffer clipping into the
>> format-helper callers.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/drm_format_helper.c | 10 ++++++++--
>>   include/drm/drm_format_helper.h     |  4 ++++
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
>> index 69fde60e36b3..28e9d0d89270 100644
>> --- a/drivers/gpu/drm/drm_format_helper.c
>> +++ b/drivers/gpu/drm/drm_format_helper.c
>> @@ -17,12 +17,18 @@
>>   #include <drm/drm_fourcc.h>
>>   #include <drm/drm_rect.h>
>>   
>> -static unsigned int clip_offset(struct drm_rect *clip,
>> -				unsigned int pitch, unsigned int cpp)
>> +static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
>>   {
>>   	return clip->y1 * pitch + clip->x1 * cpp;
>>   }
>>   
>> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
>> +				 const struct drm_rect *clip)
>> +{
>> +	return clip_offset(clip, pitch, format->cpp[0]);
>> +}
>> +EXPORT_SYMBOL(drm_fb_clip_offset);
> 
> Exported functions are expected to have kernel-doc documentation.
> Just copy more or less from the changelog I think.

That's an oversight. Sorry.

> 
> Anywhere else (I looked in struct drm_framebuffer) we only need unsigned
> int for offsets and width/length - so I cannot see why we do an unsigned
> int => unsigned long conversion here.

On ancient platforms, int was 16 bit wide. So for values that are array 
indices or buffer indices, I naturally use long, which is 32-bit at 
least. Never mind, it's not relevant any longer. I'll convert this code 
to unsigned int.

Best regards
Thomas

> 
> 	Sam
> 
>> +
>>   /**
>>    * drm_fb_memcpy - Copy clip buffer
>>    * @dst: Destination buffer
>> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
>> index e86925cf07b9..90b9bd9ecb83 100644
>> --- a/include/drm/drm_format_helper.h
>> +++ b/include/drm/drm_format_helper.h
>> @@ -6,9 +6,13 @@
>>   #ifndef __LINUX_DRM_FORMAT_HELPER_H
>>   #define __LINUX_DRM_FORMAT_HELPER_H
>>   
>> +struct drm_format_info;
>>   struct drm_framebuffer;
>>   struct drm_rect;
>>   
>> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
>> +				 const struct drm_rect *clip);
>> +
>>   void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
>>   		   struct drm_rect *clip);
>>   void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
>> -- 
>> 2.33.0

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

[Thomas Zimmermann]

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

Hi

Am 23.10.21 um 09:49 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Fri, Oct 22, 2021 at 03:28:21PM +0200, Thomas Zimmermann wrote:
>> Provide a function that computes the offset into a blit destination
>> buffer. This will allow to move destination-buffer clipping into the
>> format-helper callers.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/drm_format_helper.c | 10 ++++++++--
>>   include/drm/drm_format_helper.h     |  4 ++++
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
>> index 69fde60e36b3..28e9d0d89270 100644
>> --- a/drivers/gpu/drm/drm_format_helper.c
>> +++ b/drivers/gpu/drm/drm_format_helper.c
>> @@ -17,12 +17,18 @@
>>   #include <drm/drm_fourcc.h>
>>   #include <drm/drm_rect.h>
>>   
>> -static unsigned int clip_offset(struct drm_rect *clip,
>> -				unsigned int pitch, unsigned int cpp)
>> +static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
>>   {
>>   	return clip->y1 * pitch + clip->x1 * cpp;
>>   }
>>   
>> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
>> +				 const struct drm_rect *clip)
>> +{
>> +	return clip_offset(clip, pitch, format->cpp[0]);
>> +}
>> +EXPORT_SYMBOL(drm_fb_clip_offset);
> 
> Exported functions are expected to have kernel-doc documentation.
> Just copy more or less from the changelog I think.

That's an oversight. Sorry.

> 
> Anywhere else (I looked in struct drm_framebuffer) we only need unsigned
> int for offsets and width/length - so I cannot see why we do an unsigned
> int => unsigned long conversion here.

On ancient platforms, int was 16 bit wide. So for values that are array 
indices or buffer indices, I naturally use long, which is 32-bit at 
least. Never mind, it's not relevant any longer. I'll convert this code 
to unsigned int.

Best regards
Thomas

> 
> 	Sam
> 
>> +
>>   /**
>>    * drm_fb_memcpy - Copy clip buffer
>>    * @dst: Destination buffer
>> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
>> index e86925cf07b9..90b9bd9ecb83 100644
>> --- a/include/drm/drm_format_helper.h
>> +++ b/include/drm/drm_format_helper.h
>> @@ -6,9 +6,13 @@
>>   #ifndef __LINUX_DRM_FORMAT_HELPER_H
>>   #define __LINUX_DRM_FORMAT_HELPER_H
>>   
>> +struct drm_format_info;
>>   struct drm_framebuffer;
>>   struct drm_rect;
>>   
>> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
>> +				 const struct drm_rect *clip);
>> +
>>   void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
>>   		   struct drm_rect *clip);
>>   void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
>> -- 
>> 2.33.0

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

Re: [PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Provide a function that computes the offset into a blit destination
> buffer. This will allow to move destination-buffer clipping into the
> format-helper callers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_format_helper.c | 10 ++++++++--
>  include/drm/drm_format_helper.h     |  4 ++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 69fde60e36b3..28e9d0d89270 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -17,12 +17,18 @@
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_rect.h>
>  
> -static unsigned int clip_offset(struct drm_rect *clip,
> -				unsigned int pitch, unsigned int cpp)
> +static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
>  {
>  	return clip->y1 * pitch + clip->x1 * cpp;
>  }
>  
> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,

Like Sam I wonder about the unsigned long here.

Noralf.

> +				 const struct drm_rect *clip)
> +{
> +	return clip_offset(clip, pitch, format->cpp[0]);
> +}
> +EXPORT_SYMBOL(drm_fb_clip_offset);
> +

Re: [PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Provide a function that computes the offset into a blit destination
> buffer. This will allow to move destination-buffer clipping into the
> format-helper callers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_format_helper.c | 10 ++++++++--
>  include/drm/drm_format_helper.h     |  4 ++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 69fde60e36b3..28e9d0d89270 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -17,12 +17,18 @@
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_rect.h>
>  
> -static unsigned int clip_offset(struct drm_rect *clip,
> -				unsigned int pitch, unsigned int cpp)
> +static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
>  {
>  	return clip->y1 * pitch + clip->x1 * cpp;
>  }
>  
> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,

Like Sam I wonder about the unsigned long here.

Noralf.

> +				 const struct drm_rect *clip)
> +{
> +	return clip_offset(clip, pitch, format->cpp[0]);
> +}
> +EXPORT_SYMBOL(drm_fb_clip_offset);
> +

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 2/9] drm/format-helper: Rework format-helper memcpy functions

[Thomas Zimmermann]

Move destination-buffer clipping from all format-helper memcpy
function into callers. Support destination-buffer pitch. Only
distinguish between system and I/O memory, but use same logic
everywhere.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c         | 35 ++++++++++++---------
 drivers/gpu/drm/drm_mipi_dbi.c              |  2 +-
 drivers/gpu/drm/gud/gud_pipe.c              |  2 +-
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |  5 ++-
 drivers/gpu/drm/mgag200/mgag200_mode.c      |  4 ++-
 drivers/gpu/drm/tiny/cirrus.c               | 14 +++++----
 include/drm/drm_format_helper.h             |  9 +++---
 7 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 28e9d0d89270..38c8055f6fa8 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -32,58 +32,62 @@ EXPORT_SYMBOL(drm_fb_clip_offset);
 /**
  * drm_fb_memcpy - Copy clip buffer
  * @dst: Destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: Source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
+ * is at the top-left corner.
  */
-void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
-		   struct drm_rect *clip)
+void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr,
+		   const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	unsigned int cpp = fb->format->cpp[0];
 	size_t len = (clip->x2 - clip->x1) * cpp;
 	unsigned int y, lines = clip->y2 - clip->y1;
 
+	if (!dst_pitch)
+		dst_pitch = len;
+
 	vaddr += clip_offset(clip, fb->pitches[0], cpp);
 	for (y = 0; y < lines; y++) {
 		memcpy(dst, vaddr, len);
 		vaddr += fb->pitches[0];
-		dst += len;
+		dst += dst_pitch;
 	}
 }
 EXPORT_SYMBOL(drm_fb_memcpy);
 
 /**
- * drm_fb_memcpy_dstclip - Copy clip buffer
+ * drm_fb_memcpy_toio - Copy clip buffer
  * @dst: Destination buffer (iomem)
  * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: Source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
- * This function applies clipping on dst, i.e. the destination is a
- * full (iomem) framebuffer but only the clip rect content is copied over.
+ * This function does not apply clipping on dst, i.e. the destination
+ * is at the top-left corner.
  */
-void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			   void *vaddr, struct drm_framebuffer *fb,
-			   struct drm_rect *clip)
+void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr,
+			const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	unsigned int cpp = fb->format->cpp[0];
-	unsigned int offset = clip_offset(clip, dst_pitch, cpp);
 	size_t len = (clip->x2 - clip->x1) * cpp;
 	unsigned int y, lines = clip->y2 - clip->y1;
 
-	vaddr += offset;
-	dst += offset;
+	if (!dst_pitch)
+		dst_pitch = len;
+
+	vaddr += clip_offset(clip, fb->pitches[0], cpp);
 	for (y = 0; y < lines; y++) {
 		memcpy_toio(dst, vaddr, len);
 		vaddr += fb->pitches[0];
 		dst += dst_pitch;
 	}
 }
-EXPORT_SYMBOL(drm_fb_memcpy_dstclip);
+EXPORT_SYMBOL(drm_fb_memcpy_toio);
 
 /**
  * drm_fb_swab - Swap bytes into clip buffer
@@ -472,7 +476,8 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 		dst_format = DRM_FORMAT_XRGB8888;
 
 	if (dst_format == fb_format) {
-		drm_fb_memcpy_dstclip(dst, dst_pitch, vmap, fb, clip);
+		dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
+		drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip);
 		return 0;
 
 	} else if (dst_format == DRM_FORMAT_RGB565) {
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 71b646c4131f..c09df8b06c7a 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -213,7 +213,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 		if (swap)
 			drm_fb_swab(dst, src, fb, clip, !gem->import_attach);
 		else
-			drm_fb_memcpy(dst, src, fb, clip);
+			drm_fb_memcpy(dst, 0, src, fb, clip);
 		break;
 	case DRM_FORMAT_XRGB8888:
 		drm_fb_xrgb8888_to_rgb565(dst, src, fb, clip, swap);
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index daf75c178c2b..a92112ffd37a 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -206,7 +206,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 		/* can compress directly from the framebuffer */
 		buf = vaddr + rect->y1 * pitch;
 	} else {
-		drm_fb_memcpy(buf, vaddr, fb, rect);
+		drm_fb_memcpy(buf, 0, vaddr, fb, rect);
 	}
 
 	memset(req, 0, sizeof(*req));
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
index 8c97a20dfe23..93f51e70a951 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
@@ -23,13 +23,16 @@ static int hyperv_blit_to_vram_rect(struct drm_framebuffer *fb,
 				    struct drm_rect *rect)
 {
 	struct hyperv_drm_device *hv = to_hv(fb->dev);
+	void __iomem *dst = hv->vram;
 	void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */
 	int idx;
 
 	if (!drm_dev_enter(&hv->dev, &idx))
 		return -ENODEV;
 
-	drm_fb_memcpy_dstclip(hv->vram, fb->pitches[0], vmap, fb, rect);
+	dst += drm_fb_clip_offset(fb->pitches[0], fb->format, rect);
+	drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, rect);
+
 	drm_dev_exit(idx);
 
 	return 0;
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index fd98e8bbc550..b983541a4c53 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -847,9 +847,11 @@ static void
 mgag200_handle_damage(struct mga_device *mdev, struct drm_framebuffer *fb,
 		      struct drm_rect *clip, const struct dma_buf_map *map)
 {
+	void __iomem *dst = mdev->vram;
 	void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */
 
-	drm_fb_memcpy_dstclip(mdev->vram, fb->pitches[0], vmap, fb, clip);
+	dst += drm_fb_clip_offset(fb->pitches[0], fb->format, clip);
+	drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, clip);
 
 	/* Always scanout image at VRAM offset 0 */
 	mgag200_set_startadd(mdev, (u32)0);
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 4611ec408506..5344e506e8a9 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -317,28 +317,30 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, const struct dma_buf_
 			       struct drm_rect *rect)
 {
 	struct cirrus_device *cirrus = to_cirrus(fb->dev);
+	void __iomem *dst = cirrus->vram;
 	void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */
 	int idx;
 
 	if (!drm_dev_enter(&cirrus->dev, &idx))
 		return -ENODEV;
 
-	if (cirrus->cpp == fb->format->cpp[0])
-		drm_fb_memcpy_dstclip(cirrus->vram, fb->pitches[0],
-				      vmap, fb, rect);
+	if (cirrus->cpp == fb->format->cpp[0]) {
+		dst += drm_fb_clip_offset(fb->pitches[0], fb->format, rect);
+		drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, rect);
 
-	else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2)
+	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2) {
 		drm_fb_xrgb8888_to_rgb565_dstclip(cirrus->vram,
 						  cirrus->pitch,
 						  vmap, fb, rect, false);
 
-	else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3)
+	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3) {
 		drm_fb_xrgb8888_to_rgb888_dstclip(cirrus->vram,
 						  cirrus->pitch,
 						  vmap, fb, rect);
 
-	else
+	} else {
 		WARN_ON_ONCE("cpp mismatch");
+	}
 
 	drm_dev_exit(idx);
 
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 90b9bd9ecb83..8d72f6fd27e9 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -13,11 +13,10 @@ struct drm_rect;
 unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
 				 const struct drm_rect *clip);
 
-void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
-		   struct drm_rect *clip);
-void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
-			   struct drm_framebuffer *fb,
-			   struct drm_rect *clip);
+void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr,
+		   const struct drm_framebuffer *fb, const struct drm_rect *clip);
+void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr,
+			const struct drm_framebuffer *fb, const struct drm_rect *clip);
 void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
 		 struct drm_rect *clip, bool cached);
 void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb,
-- 
2.33.0

[PATCH 2/9] drm/format-helper: Rework format-helper memcpy functions

[Thomas Zimmermann]

Move destination-buffer clipping from all format-helper memcpy
function into callers. Support destination-buffer pitch. Only
distinguish between system and I/O memory, but use same logic
everywhere.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c         | 35 ++++++++++++---------
 drivers/gpu/drm/drm_mipi_dbi.c              |  2 +-
 drivers/gpu/drm/gud/gud_pipe.c              |  2 +-
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |  5 ++-
 drivers/gpu/drm/mgag200/mgag200_mode.c      |  4 ++-
 drivers/gpu/drm/tiny/cirrus.c               | 14 +++++----
 include/drm/drm_format_helper.h             |  9 +++---
 7 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 28e9d0d89270..38c8055f6fa8 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -32,58 +32,62 @@ EXPORT_SYMBOL(drm_fb_clip_offset);
 /**
  * drm_fb_memcpy - Copy clip buffer
  * @dst: Destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: Source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
+ * is at the top-left corner.
  */
-void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
-		   struct drm_rect *clip)
+void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr,
+		   const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	unsigned int cpp = fb->format->cpp[0];
 	size_t len = (clip->x2 - clip->x1) * cpp;
 	unsigned int y, lines = clip->y2 - clip->y1;
 
+	if (!dst_pitch)
+		dst_pitch = len;
+
 	vaddr += clip_offset(clip, fb->pitches[0], cpp);
 	for (y = 0; y < lines; y++) {
 		memcpy(dst, vaddr, len);
 		vaddr += fb->pitches[0];
-		dst += len;
+		dst += dst_pitch;
 	}
 }
 EXPORT_SYMBOL(drm_fb_memcpy);
 
 /**
- * drm_fb_memcpy_dstclip - Copy clip buffer
+ * drm_fb_memcpy_toio - Copy clip buffer
  * @dst: Destination buffer (iomem)
  * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: Source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
- * This function applies clipping on dst, i.e. the destination is a
- * full (iomem) framebuffer but only the clip rect content is copied over.
+ * This function does not apply clipping on dst, i.e. the destination
+ * is at the top-left corner.
  */
-void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			   void *vaddr, struct drm_framebuffer *fb,
-			   struct drm_rect *clip)
+void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr,
+			const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	unsigned int cpp = fb->format->cpp[0];
-	unsigned int offset = clip_offset(clip, dst_pitch, cpp);
 	size_t len = (clip->x2 - clip->x1) * cpp;
 	unsigned int y, lines = clip->y2 - clip->y1;
 
-	vaddr += offset;
-	dst += offset;
+	if (!dst_pitch)
+		dst_pitch = len;
+
+	vaddr += clip_offset(clip, fb->pitches[0], cpp);
 	for (y = 0; y < lines; y++) {
 		memcpy_toio(dst, vaddr, len);
 		vaddr += fb->pitches[0];
 		dst += dst_pitch;
 	}
 }
-EXPORT_SYMBOL(drm_fb_memcpy_dstclip);
+EXPORT_SYMBOL(drm_fb_memcpy_toio);
 
 /**
  * drm_fb_swab - Swap bytes into clip buffer
@@ -472,7 +476,8 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 		dst_format = DRM_FORMAT_XRGB8888;
 
 	if (dst_format == fb_format) {
-		drm_fb_memcpy_dstclip(dst, dst_pitch, vmap, fb, clip);
+		dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
+		drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip);
 		return 0;
 
 	} else if (dst_format == DRM_FORMAT_RGB565) {
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 71b646c4131f..c09df8b06c7a 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -213,7 +213,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 		if (swap)
 			drm_fb_swab(dst, src, fb, clip, !gem->import_attach);
 		else
-			drm_fb_memcpy(dst, src, fb, clip);
+			drm_fb_memcpy(dst, 0, src, fb, clip);
 		break;
 	case DRM_FORMAT_XRGB8888:
 		drm_fb_xrgb8888_to_rgb565(dst, src, fb, clip, swap);
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index daf75c178c2b..a92112ffd37a 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -206,7 +206,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 		/* can compress directly from the framebuffer */
 		buf = vaddr + rect->y1 * pitch;
 	} else {
-		drm_fb_memcpy(buf, vaddr, fb, rect);
+		drm_fb_memcpy(buf, 0, vaddr, fb, rect);
 	}
 
 	memset(req, 0, sizeof(*req));
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
index 8c97a20dfe23..93f51e70a951 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
@@ -23,13 +23,16 @@ static int hyperv_blit_to_vram_rect(struct drm_framebuffer *fb,
 				    struct drm_rect *rect)
 {
 	struct hyperv_drm_device *hv = to_hv(fb->dev);
+	void __iomem *dst = hv->vram;
 	void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */
 	int idx;
 
 	if (!drm_dev_enter(&hv->dev, &idx))
 		return -ENODEV;
 
-	drm_fb_memcpy_dstclip(hv->vram, fb->pitches[0], vmap, fb, rect);
+	dst += drm_fb_clip_offset(fb->pitches[0], fb->format, rect);
+	drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, rect);
+
 	drm_dev_exit(idx);
 
 	return 0;
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index fd98e8bbc550..b983541a4c53 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -847,9 +847,11 @@ static void
 mgag200_handle_damage(struct mga_device *mdev, struct drm_framebuffer *fb,
 		      struct drm_rect *clip, const struct dma_buf_map *map)
 {
+	void __iomem *dst = mdev->vram;
 	void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */
 
-	drm_fb_memcpy_dstclip(mdev->vram, fb->pitches[0], vmap, fb, clip);
+	dst += drm_fb_clip_offset(fb->pitches[0], fb->format, clip);
+	drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, clip);
 
 	/* Always scanout image at VRAM offset 0 */
 	mgag200_set_startadd(mdev, (u32)0);
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 4611ec408506..5344e506e8a9 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -317,28 +317,30 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, const struct dma_buf_
 			       struct drm_rect *rect)
 {
 	struct cirrus_device *cirrus = to_cirrus(fb->dev);
+	void __iomem *dst = cirrus->vram;
 	void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */
 	int idx;
 
 	if (!drm_dev_enter(&cirrus->dev, &idx))
 		return -ENODEV;
 
-	if (cirrus->cpp == fb->format->cpp[0])
-		drm_fb_memcpy_dstclip(cirrus->vram, fb->pitches[0],
-				      vmap, fb, rect);
+	if (cirrus->cpp == fb->format->cpp[0]) {
+		dst += drm_fb_clip_offset(fb->pitches[0], fb->format, rect);
+		drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, rect);
 
-	else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2)
+	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2) {
 		drm_fb_xrgb8888_to_rgb565_dstclip(cirrus->vram,
 						  cirrus->pitch,
 						  vmap, fb, rect, false);
 
-	else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3)
+	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3) {
 		drm_fb_xrgb8888_to_rgb888_dstclip(cirrus->vram,
 						  cirrus->pitch,
 						  vmap, fb, rect);
 
-	else
+	} else {
 		WARN_ON_ONCE("cpp mismatch");
+	}
 
 	drm_dev_exit(idx);
 
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 90b9bd9ecb83..8d72f6fd27e9 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -13,11 +13,10 @@ struct drm_rect;
 unsigned long drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
 				 const struct drm_rect *clip);
 
-void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
-		   struct drm_rect *clip);
-void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr,
-			   struct drm_framebuffer *fb,
-			   struct drm_rect *clip);
+void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr,
+		   const struct drm_framebuffer *fb, const struct drm_rect *clip);
+void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr,
+			const struct drm_framebuffer *fb, const struct drm_rect *clip);
 void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
 		 struct drm_rect *clip, bool cached);
 void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb,
-- 
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/9] drm/format-helper: Rework format-helper memcpy functions

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Move destination-buffer clipping from all format-helper memcpy
> function into callers. Support destination-buffer pitch. Only
> distinguish between system and I/O memory, but use same logic
> everywhere.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Tested-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

Re: [PATCH 2/9] drm/format-helper: Rework format-helper memcpy functions

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Move destination-buffer clipping from all format-helper memcpy
> function into callers. Support destination-buffer pitch. Only
> distinguish between system and I/O memory, but use same logic
> everywhere.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Tested-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 3/9] drm/format-helper: Add destination-buffer pitch to drm_fb_swab()

[Thomas Zimmermann]

Add destination-buffer pitch as argument to drm_fb_swab(). Done for
consistency with the rest of the interface.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 19 +++++++++++++++----
 drivers/gpu/drm/drm_mipi_dbi.c      |  2 +-
 drivers/gpu/drm/gud/gud_pipe.c      |  2 +-
 include/drm/drm_format_helper.h     |  5 +++--
 4 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 38c8055f6fa8..79869ed553d9 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -92,6 +92,7 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
 /**
  * drm_fb_swab - Swap bytes into clip buffer
  * @dst: Destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @src: Source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -103,19 +104,25 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
  * This function does not apply clipping on dst, i.e. the destination
  * is a small buffer containing the clip rect only.
  */
-void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
-		 struct drm_rect *clip, bool cached)
+void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
+		 const struct drm_framebuffer *fb, const struct drm_rect *clip,
+		 bool cached)
 {
 	u8 cpp = fb->format->cpp[0];
 	size_t len = drm_rect_width(clip) * cpp;
-	u16 *src16, *dst16 = dst;
-	u32 *src32, *dst32 = dst;
+	const u16 *src16;
+	const u32 *src32;
+	u16 *dst16;
+	u32 *dst32;
 	unsigned int x, y;
 	void *buf = NULL;
 
 	if (WARN_ON_ONCE(cpp != 2 && cpp != 4))
 		return;
 
+	if (!dst_pitch)
+		dst_pitch = len;
+
 	if (!cached)
 		buf = kmalloc(len, GFP_KERNEL);
 
@@ -131,6 +138,9 @@ void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
 			src32 = src;
 		}
 
+		dst16 = dst;
+		dst32 = dst;
+
 		for (x = clip->x1; x < clip->x2; x++) {
 			if (cpp == 4)
 				*dst32++ = swab32(*src32++);
@@ -139,6 +149,7 @@ void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
 		}
 
 		src += fb->pitches[0];
+		dst += dst_pitch;
 	}
 
 	kfree(buf);
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index c09df8b06c7a..7ce89917d761 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -211,7 +211,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 	switch (fb->format->format) {
 	case DRM_FORMAT_RGB565:
 		if (swap)
-			drm_fb_swab(dst, src, fb, clip, !gem->import_attach);
+			drm_fb_swab(dst, 0, src, fb, clip, !gem->import_attach);
 		else
 			drm_fb_memcpy(dst, 0, src, fb, clip);
 		break;
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index a92112ffd37a..e0b117b2559f 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -201,7 +201,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 			len = gud_xrgb8888_to_color(buf, format, vaddr, fb, rect);
 		}
 	} else if (gud_is_big_endian() && format->cpp[0] > 1) {
-		drm_fb_swab(buf, vaddr, fb, rect, !import_attach);
+		drm_fb_swab(buf, 0, vaddr, fb, rect, !import_attach);
 	} else if (compression && !import_attach && pitch == fb->pitches[0]) {
 		/* can compress directly from the framebuffer */
 		buf = vaddr + rect->y1 * pitch;
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 8d72f6fd27e9..c4c3c845e119 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -17,8 +17,9 @@ void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr,
 		   const struct drm_framebuffer *fb, const struct drm_rect *clip);
 void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr,
 			const struct drm_framebuffer *fb, const struct drm_rect *clip);
-void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
-		 struct drm_rect *clip, bool cached);
+void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
+		 const struct drm_framebuffer *fb, const struct drm_rect *clip,
+		 bool cached);
 void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb,
 			       struct drm_rect *clip);
 void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
-- 
2.33.0

[PATCH 3/9] drm/format-helper: Add destination-buffer pitch to drm_fb_swab()

[Thomas Zimmermann]

Add destination-buffer pitch as argument to drm_fb_swab(). Done for
consistency with the rest of the interface.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 19 +++++++++++++++----
 drivers/gpu/drm/drm_mipi_dbi.c      |  2 +-
 drivers/gpu/drm/gud/gud_pipe.c      |  2 +-
 include/drm/drm_format_helper.h     |  5 +++--
 4 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 38c8055f6fa8..79869ed553d9 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -92,6 +92,7 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
 /**
  * drm_fb_swab - Swap bytes into clip buffer
  * @dst: Destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @src: Source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -103,19 +104,25 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
  * This function does not apply clipping on dst, i.e. the destination
  * is a small buffer containing the clip rect only.
  */
-void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
-		 struct drm_rect *clip, bool cached)
+void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
+		 const struct drm_framebuffer *fb, const struct drm_rect *clip,
+		 bool cached)
 {
 	u8 cpp = fb->format->cpp[0];
 	size_t len = drm_rect_width(clip) * cpp;
-	u16 *src16, *dst16 = dst;
-	u32 *src32, *dst32 = dst;
+	const u16 *src16;
+	const u32 *src32;
+	u16 *dst16;
+	u32 *dst32;
 	unsigned int x, y;
 	void *buf = NULL;
 
 	if (WARN_ON_ONCE(cpp != 2 && cpp != 4))
 		return;
 
+	if (!dst_pitch)
+		dst_pitch = len;
+
 	if (!cached)
 		buf = kmalloc(len, GFP_KERNEL);
 
@@ -131,6 +138,9 @@ void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
 			src32 = src;
 		}
 
+		dst16 = dst;
+		dst32 = dst;
+
 		for (x = clip->x1; x < clip->x2; x++) {
 			if (cpp == 4)
 				*dst32++ = swab32(*src32++);
@@ -139,6 +149,7 @@ void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
 		}
 
 		src += fb->pitches[0];
+		dst += dst_pitch;
 	}
 
 	kfree(buf);
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index c09df8b06c7a..7ce89917d761 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -211,7 +211,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 	switch (fb->format->format) {
 	case DRM_FORMAT_RGB565:
 		if (swap)
-			drm_fb_swab(dst, src, fb, clip, !gem->import_attach);
+			drm_fb_swab(dst, 0, src, fb, clip, !gem->import_attach);
 		else
 			drm_fb_memcpy(dst, 0, src, fb, clip);
 		break;
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index a92112ffd37a..e0b117b2559f 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -201,7 +201,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 			len = gud_xrgb8888_to_color(buf, format, vaddr, fb, rect);
 		}
 	} else if (gud_is_big_endian() && format->cpp[0] > 1) {
-		drm_fb_swab(buf, vaddr, fb, rect, !import_attach);
+		drm_fb_swab(buf, 0, vaddr, fb, rect, !import_attach);
 	} else if (compression && !import_attach && pitch == fb->pitches[0]) {
 		/* can compress directly from the framebuffer */
 		buf = vaddr + rect->y1 * pitch;
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 8d72f6fd27e9..c4c3c845e119 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -17,8 +17,9 @@ void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr,
 		   const struct drm_framebuffer *fb, const struct drm_rect *clip);
 void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr,
 			const struct drm_framebuffer *fb, const struct drm_rect *clip);
-void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
-		 struct drm_rect *clip, bool cached);
+void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
+		 const struct drm_framebuffer *fb, const struct drm_rect *clip,
+		 bool cached);
 void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb,
 			       struct drm_rect *clip);
 void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
-- 
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 3/9] drm/format-helper: Add destination-buffer pitch to drm_fb_swab()

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Add destination-buffer pitch as argument to drm_fb_swab(). Done for
> consistency with the rest of the interface.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_format_helper.c | 19 +++++++++++++++----
>  drivers/gpu/drm/drm_mipi_dbi.c      |  2 +-
>  drivers/gpu/drm/gud/gud_pipe.c      |  2 +-
>  include/drm/drm_format_helper.h     |  5 +++--
>  4 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 38c8055f6fa8..79869ed553d9 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -92,6 +92,7 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
>  /**
>   * drm_fb_swab - Swap bytes into clip buffer
>   * @dst: Destination buffer
> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
>   * @src: Source buffer
>   * @fb: DRM framebuffer
>   * @clip: Clip rectangle area to copy
> @@ -103,19 +104,25 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
>   * This function does not apply clipping on dst, i.e. the destination

You have changed this line on the other functions, maybe you just missed
it here:

>   * is a small buffer containing the clip rect only.
>   */
> -void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
> -		 struct drm_rect *clip, bool cached)
> +void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
> +		 const struct drm_framebuffer *fb, const struct drm_rect *clip,
> +		 bool cached)

Tested-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

Re: [PATCH 3/9] drm/format-helper: Add destination-buffer pitch to drm_fb_swab()

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Add destination-buffer pitch as argument to drm_fb_swab(). Done for
> consistency with the rest of the interface.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_format_helper.c | 19 +++++++++++++++----
>  drivers/gpu/drm/drm_mipi_dbi.c      |  2 +-
>  drivers/gpu/drm/gud/gud_pipe.c      |  2 +-
>  include/drm/drm_format_helper.h     |  5 +++--
>  4 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 38c8055f6fa8..79869ed553d9 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -92,6 +92,7 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
>  /**
>   * drm_fb_swab - Swap bytes into clip buffer
>   * @dst: Destination buffer
> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
>   * @src: Source buffer
>   * @fb: DRM framebuffer
>   * @clip: Clip rectangle area to copy
> @@ -103,19 +104,25 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
>   * This function does not apply clipping on dst, i.e. the destination

You have changed this line on the other functions, maybe you just missed
it here:

>   * is a small buffer containing the clip rect only.
>   */
> -void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
> -		 struct drm_rect *clip, bool cached)
> +void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
> +		 const struct drm_framebuffer *fb, const struct drm_rect *clip,
> +		 bool cached)

Tested-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 4/9] drm/format-helper: Rework format-helper conversion functions

[Thomas Zimmermann]

Move destination-buffer clipping from all format-helper conversion
functions into callers. Support destination-buffer pitch. Only
distinguish between system and I/O memory, but use same logic
everywhere.

Simply harmonize the interface and semantics of the existing code.
Not all conversion helpers support all combinations of parameters.
We have to add additional features when we need them.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 131 +++++++++++++++-------------
 drivers/gpu/drm/drm_mipi_dbi.c      |   2 +-
 drivers/gpu/drm/gud/gud_pipe.c      |  10 +--
 drivers/gpu/drm/tiny/cirrus.c       |  10 +--
 drivers/gpu/drm/tiny/repaper.c      |   2 +-
 drivers/gpu/drm/tiny/st7586.c       |   2 +-
 include/drm/drm_format_helper.h     |  30 +++----
 7 files changed, 97 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 79869ed553d9..260dc587c1d7 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -156,7 +156,7 @@ void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
 }
 EXPORT_SYMBOL(drm_fb_swab);
 
-static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned int pixels)
+static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, const __le32 *sbuf, unsigned int pixels)
 {
 	unsigned int x;
 	u32 pix;
@@ -172,23 +172,24 @@ static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned int
 /**
  * drm_fb_xrgb8888_to_rgb332 - Convert XRGB8888 to RGB332 clip buffer
  * @dst: RGB332 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @src: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * Drivers can use this function for RGB332 devices that don't natively support XRGB8888.
- *
- * This function does not apply clipping on dst, i.e. the destination is a small buffer
- * containing the clip rect only.
  */
-void drm_fb_xrgb8888_to_rgb332(void *dst, void *src, struct drm_framebuffer *fb,
-			       struct drm_rect *clip)
+void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *src,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	size_t width = drm_rect_width(clip);
 	size_t src_len = width * sizeof(u32);
 	unsigned int y;
 	void *sbuf;
 
+	if (!dst_pitch)
+		dst_pitch = width;
+
 	/* Use a buffer to speed up access on buffers with uncached read mapping (i.e. WC) */
 	sbuf = kmalloc(src_len, GFP_KERNEL);
 	if (!sbuf)
@@ -199,14 +200,14 @@ void drm_fb_xrgb8888_to_rgb332(void *dst, void *src, struct drm_framebuffer *fb,
 		memcpy(sbuf, src, src_len);
 		drm_fb_xrgb8888_to_rgb332_line(dst, sbuf, width);
 		src += fb->pitches[0];
-		dst += width;
+		dst += dst_pitch;
 	}
 
 	kfree(sbuf);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
 
-static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf,
+static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, const u32 *sbuf,
 					   unsigned int pixels,
 					   bool swab)
 {
@@ -227,6 +228,7 @@ static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf,
 /**
  * drm_fb_xrgb8888_to_rgb565 - Convert XRGB8888 to RGB565 clip buffer
  * @dst: RGB565 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -234,13 +236,10 @@ static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf,
  *
  * Drivers can use this function for RGB565 devices that don't natively
  * support XRGB8888.
- *
- * This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
  */
-void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
-			       struct drm_framebuffer *fb,
-			       struct drm_rect *clip, bool swab)
+void drm_fb_xrgb8888_to_rgb565(void *dst, unsigned int dst_pitch, const void *vaddr,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip,
+			       bool swab)
 {
 	size_t linepixels = clip->x2 - clip->x1;
 	size_t src_len = linepixels * sizeof(u32);
@@ -248,6 +247,9 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
 	unsigned y, lines = clip->y2 - clip->y1;
 	void *sbuf;
 
+	if (!dst_pitch)
+		dst_pitch = dst_len;
+
 	/*
 	 * The cma memory is write-combined so reads are uncached.
 	 * Speed up by fetching one line at a time.
@@ -261,7 +263,7 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
 		memcpy(sbuf, vaddr, src_len);
 		drm_fb_xrgb8888_to_rgb565_line(dst, sbuf, linepixels, swab);
 		vaddr += fb->pitches[0];
-		dst += dst_len;
+		dst += dst_pitch;
 	}
 
 	kfree(sbuf);
@@ -269,9 +271,9 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
 
 /**
- * drm_fb_xrgb8888_to_rgb565_dstclip - Convert XRGB8888 to RGB565 clip buffer
+ * drm_fb_xrgb8888_to_rgb565_toio - Convert XRGB8888 to RGB565 clip buffer
  * @dst: RGB565 destination buffer (iomem)
- * @dst_pitch: destination buffer pitch
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -279,37 +281,36 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
  *
  * Drivers can use this function for RGB565 devices that don't natively
  * support XRGB8888.
- *
- * This function applies clipping on dst, i.e. the destination is a
- * full (iomem) framebuffer but only the clip rect content is copied over.
  */
-void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst, unsigned int dst_pitch,
-				       void *vaddr, struct drm_framebuffer *fb,
-				       struct drm_rect *clip, bool swab)
+void drm_fb_xrgb8888_to_rgb565_toio(void __iomem *dst, unsigned int dst_pitch,
+				    const void *vaddr, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip, bool swab)
 {
 	size_t linepixels = clip->x2 - clip->x1;
 	size_t dst_len = linepixels * sizeof(u16);
 	unsigned y, lines = clip->y2 - clip->y1;
 	void *dbuf;
 
+	if (!dst_pitch)
+		dst_pitch = dst_len;
+
 	dbuf = kmalloc(dst_len, GFP_KERNEL);
 	if (!dbuf)
 		return;
 
 	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
-	dst += clip_offset(clip, dst_pitch, sizeof(u16));
 	for (y = 0; y < lines; y++) {
 		drm_fb_xrgb8888_to_rgb565_line(dbuf, vaddr, linepixels, swab);
 		memcpy_toio(dst, dbuf, dst_len);
 		vaddr += fb->pitches[0];
-		dst += dst_len;
+		dst += dst_pitch;
 	}
 
 	kfree(dbuf);
 }
-EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_dstclip);
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_toio);
 
-static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, u32 *sbuf,
+static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, const u32 *sbuf,
 					   unsigned int pixels)
 {
 	unsigned int x;
@@ -324,24 +325,25 @@ static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, u32 *sbuf,
 /**
  * drm_fb_xrgb8888_to_rgb888 - Convert XRGB8888 to RGB888 clip buffer
  * @dst: RGB888 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @src: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * Drivers can use this function for RGB888 devices that don't natively
  * support XRGB8888.
- *
- * This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
  */
-void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
-			       struct drm_rect *clip)
+void drm_fb_xrgb8888_to_rgb888(void *dst, unsigned int dst_pitch, const void *src,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	size_t width = drm_rect_width(clip);
 	size_t src_len = width * sizeof(u32);
 	unsigned int y;
 	void *sbuf;
 
+	if (!dst_pitch)
+		dst_pitch = width * 3;
+
 	/* Use a buffer to speed up access on buffers with uncached read mapping (i.e. WC) */
 	sbuf = kmalloc(src_len, GFP_KERNEL);
 	if (!sbuf)
@@ -352,7 +354,7 @@ void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
 		memcpy(sbuf, src, src_len);
 		drm_fb_xrgb8888_to_rgb888_line(dst, sbuf, width);
 		src += fb->pitches[0];
-		dst += width * 3;
+		dst += dst_pitch;
 	}
 
 	kfree(sbuf);
@@ -360,48 +362,48 @@ void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
 
 /**
- * drm_fb_xrgb8888_to_rgb888_dstclip - Convert XRGB8888 to RGB888 clip buffer
+ * drm_fb_xrgb8888_to_rgb888_toio - Convert XRGB8888 to RGB888 clip buffer
  * @dst: RGB565 destination buffer (iomem)
- * @dst_pitch: destination buffer pitch
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * Drivers can use this function for RGB888 devices that don't natively
  * support XRGB8888.
- *
- * This function applies clipping on dst, i.e. the destination is a
- * full (iomem) framebuffer but only the clip rect content is copied over.
  */
-void drm_fb_xrgb8888_to_rgb888_dstclip(void __iomem *dst, unsigned int dst_pitch,
-				       void *vaddr, struct drm_framebuffer *fb,
-				       struct drm_rect *clip)
+void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch,
+				    const void *vaddr, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip)
 {
 	size_t linepixels = clip->x2 - clip->x1;
 	size_t dst_len = linepixels * 3;
 	unsigned y, lines = clip->y2 - clip->y1;
 	void *dbuf;
 
+	if (!dst_pitch)
+		dst_pitch = dst_len;
+
 	dbuf = kmalloc(dst_len, GFP_KERNEL);
 	if (!dbuf)
 		return;
 
 	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
-	dst += clip_offset(clip, dst_pitch, sizeof(u16));
 	for (y = 0; y < lines; y++) {
 		drm_fb_xrgb8888_to_rgb888_line(dbuf, vaddr, linepixels);
 		memcpy_toio(dst, dbuf, dst_len);
 		vaddr += fb->pitches[0];
-		dst += dst_len;
+		dst += dst_pitch;
 	}
 
 	kfree(dbuf);
 }
-EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_toio);
 
 /**
  * drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
  * @dst: 8-bit grayscale destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
  *
  * ITU BT.601 is used for the RGB -> luma (brightness) conversion.
  */
-void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
-			       struct drm_rect *clip)
+void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
+			      const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	unsigned int len = (clip->x2 - clip->x1) * sizeof(u32);
 	unsigned int x, y;
 	void *buf;
-	u32 *src;
+	u8 *dst8;
+	u32 *src32;
 
 	if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
 		return;
+
+	if (!dst_pitch)
+		dst_pitch = len;
+
 	/*
 	 * The cma memory is write-combined so reads are uncached.
 	 * Speed up by fetching one line at a time.
@@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
 	if (!buf)
 		return;
 
+	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
 	for (y = clip->y1; y < clip->y2; y++) {
-		src = vaddr + (y * fb->pitches[0]);
-		src += clip->x1;
-		memcpy(buf, src, len);
-		src = buf;
+		dst8 = dst;
+		src32 = memcpy(buf, vaddr, len);
 		for (x = clip->x1; x < clip->x2; x++) {
-			u8 r = (*src & 0x00ff0000) >> 16;
-			u8 g = (*src & 0x0000ff00) >> 8;
-			u8 b =  *src & 0x000000ff;
+			u8 r = (*src32 & 0x00ff0000) >> 16;
+			u8 g = (*src32 & 0x0000ff00) >> 8;
+			u8 b =  *src32 & 0x000000ff;
 
 			/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
-			*dst++ = (3 * r + 6 * g + b) / 10;
-			src++;
+			*dst8++ = (3 * r + 6 * g + b) / 10;
+			src32++;
 		}
+
+		vaddr += fb->pitches[0];
+		dst += dst_pitch;
 	}
 
 	kfree(buf);
@@ -493,15 +502,15 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 
 	} else if (dst_format == DRM_FORMAT_RGB565) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
-			drm_fb_xrgb8888_to_rgb565_dstclip(dst, dst_pitch,
-							  vmap, fb, clip,
-							  false);
+			dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
+			drm_fb_xrgb8888_to_rgb565_toio(dst, dst_pitch, vmap, fb, clip,
+						       false);
 			return 0;
 		}
 	} else if (dst_format == DRM_FORMAT_RGB888) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
-			drm_fb_xrgb8888_to_rgb888_dstclip(dst, dst_pitch,
-							  vmap, fb, clip);
+			dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
+			drm_fb_xrgb8888_to_rgb888_toio(dst, dst_pitch, vmap, fb, clip);
 			return 0;
 		}
 	}
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 7ce89917d761..b75403f3251a 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -216,7 +216,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 			drm_fb_memcpy(dst, 0, src, fb, clip);
 		break;
 	case DRM_FORMAT_XRGB8888:
-		drm_fb_xrgb8888_to_rgb565(dst, src, fb, clip, swap);
+		drm_fb_xrgb8888_to_rgb565(dst, 0, src, fb, clip, swap);
 		break;
 	default:
 		drm_err_once(fb->dev, "Format is not supported: %p4cc\n",
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index e0b117b2559f..a150a5a4b5d4 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -74,7 +74,7 @@ static size_t gud_xrgb8888_to_r124(u8 *dst, const struct drm_format_info *format
 	if (!buf)
 		return 0;
 
-	drm_fb_xrgb8888_to_gray8(buf, src, fb, rect);
+	drm_fb_xrgb8888_to_gray8(buf, 0, src, fb, rect);
 	pix8 = buf;
 
 	for (y = 0; y < height; y++) {
@@ -190,13 +190,13 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 				goto end_cpu_access;
 			}
 		} else if (format->format == DRM_FORMAT_R8) {
-			drm_fb_xrgb8888_to_gray8(buf, vaddr, fb, rect);
+			drm_fb_xrgb8888_to_gray8(buf, 0, vaddr, fb, rect);
 		} else if (format->format == DRM_FORMAT_RGB332) {
-			drm_fb_xrgb8888_to_rgb332(buf, vaddr, fb, rect);
+			drm_fb_xrgb8888_to_rgb332(buf, 0, vaddr, fb, rect);
 		} else if (format->format == DRM_FORMAT_RGB565) {
-			drm_fb_xrgb8888_to_rgb565(buf, vaddr, fb, rect, gud_is_big_endian());
+			drm_fb_xrgb8888_to_rgb565(buf, 0, vaddr, fb, rect, gud_is_big_endian());
 		} else if (format->format == DRM_FORMAT_RGB888) {
-			drm_fb_xrgb8888_to_rgb888(buf, vaddr, fb, rect);
+			drm_fb_xrgb8888_to_rgb888(buf, 0, vaddr, fb, rect);
 		} else {
 			len = gud_xrgb8888_to_color(buf, format, vaddr, fb, rect);
 		}
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 5344e506e8a9..9327001d35dd 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -329,14 +329,12 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, const struct dma_buf_
 		drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, rect);
 
 	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2) {
-		drm_fb_xrgb8888_to_rgb565_dstclip(cirrus->vram,
-						  cirrus->pitch,
-						  vmap, fb, rect, false);
+		dst += drm_fb_clip_offset(cirrus->pitch, fb->format, rect);
+		drm_fb_xrgb8888_to_rgb565_toio(dst, cirrus->pitch, vmap, fb, rect, false);
 
 	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3) {
-		drm_fb_xrgb8888_to_rgb888_dstclip(cirrus->vram,
-						  cirrus->pitch,
-						  vmap, fb, rect);
+		dst += drm_fb_clip_offset(cirrus->pitch, fb->format, rect);
+		drm_fb_xrgb8888_to_rgb888_toio(dst, cirrus->pitch, vmap, fb, rect);
 
 	} else {
 		WARN_ON_ONCE("cpp mismatch");
diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index 4d07b21a16e6..97a775c48cea 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -560,7 +560,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
 	if (ret)
 		goto out_free;
 
-	drm_fb_xrgb8888_to_gray8(buf, cma_obj->vaddr, fb, &clip);
+	drm_fb_xrgb8888_to_gray8(buf, 0, cma_obj->vaddr, fb, &clip);
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index ad0faa8723c2..51b9b9fb3ead 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -73,7 +73,7 @@ static void st7586_xrgb8888_to_gray332(u8 *dst, void *vaddr,
 	if (!buf)
 		return;
 
-	drm_fb_xrgb8888_to_gray8(buf, vaddr, fb, clip);
+	drm_fb_xrgb8888_to_gray8(buf, 0, vaddr, fb, clip);
 	src = buf;
 
 	for (y = clip->y1; y < clip->y2; y++) {
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index c4c3c845e119..8af9b8f35dd6 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -20,21 +20,21 @@ void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *v
 void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
 		 const struct drm_framebuffer *fb, const struct drm_rect *clip,
 		 bool cached);
-void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb,
-			       struct drm_rect *clip);
-void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
-			       struct drm_framebuffer *fb,
-			       struct drm_rect *clip, bool swab);
-void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst, unsigned int dst_pitch,
-				       void *vaddr, struct drm_framebuffer *fb,
-				       struct drm_rect *clip, bool swab);
-void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
-			       struct drm_rect *clip);
-void drm_fb_xrgb8888_to_rgb888_dstclip(void __iomem *dst, unsigned int dst_pitch,
-				       void *vaddr, struct drm_framebuffer *fb,
-				       struct drm_rect *clip);
-void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
-			      struct drm_rect *clip);
+void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *vaddr,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_rgb565(void *dst, unsigned int dst_pitch, const void *vaddr,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip,
+			       bool swab);
+void drm_fb_xrgb8888_to_rgb565_toio(void __iomem *dst, unsigned int dst_pitch,
+				    const void *vaddr, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip, bool swab);
+void drm_fb_xrgb8888_to_rgb888(void *dst, unsigned int dst_pitch, const void *src,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch,
+				    const void *vaddr, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
+			      const struct drm_framebuffer *fb, const struct drm_rect *clip);
 
 int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 			     uint32_t dst_format, void *vmap,
-- 
2.33.0

[PATCH 4/9] drm/format-helper: Rework format-helper conversion functions

[Thomas Zimmermann]

Move destination-buffer clipping from all format-helper conversion
functions into callers. Support destination-buffer pitch. Only
distinguish between system and I/O memory, but use same logic
everywhere.

Simply harmonize the interface and semantics of the existing code.
Not all conversion helpers support all combinations of parameters.
We have to add additional features when we need them.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 131 +++++++++++++++-------------
 drivers/gpu/drm/drm_mipi_dbi.c      |   2 +-
 drivers/gpu/drm/gud/gud_pipe.c      |  10 +--
 drivers/gpu/drm/tiny/cirrus.c       |  10 +--
 drivers/gpu/drm/tiny/repaper.c      |   2 +-
 drivers/gpu/drm/tiny/st7586.c       |   2 +-
 include/drm/drm_format_helper.h     |  30 +++----
 7 files changed, 97 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 79869ed553d9..260dc587c1d7 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -156,7 +156,7 @@ void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
 }
 EXPORT_SYMBOL(drm_fb_swab);
 
-static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned int pixels)
+static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, const __le32 *sbuf, unsigned int pixels)
 {
 	unsigned int x;
 	u32 pix;
@@ -172,23 +172,24 @@ static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned int
 /**
  * drm_fb_xrgb8888_to_rgb332 - Convert XRGB8888 to RGB332 clip buffer
  * @dst: RGB332 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @src: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * Drivers can use this function for RGB332 devices that don't natively support XRGB8888.
- *
- * This function does not apply clipping on dst, i.e. the destination is a small buffer
- * containing the clip rect only.
  */
-void drm_fb_xrgb8888_to_rgb332(void *dst, void *src, struct drm_framebuffer *fb,
-			       struct drm_rect *clip)
+void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *src,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	size_t width = drm_rect_width(clip);
 	size_t src_len = width * sizeof(u32);
 	unsigned int y;
 	void *sbuf;
 
+	if (!dst_pitch)
+		dst_pitch = width;
+
 	/* Use a buffer to speed up access on buffers with uncached read mapping (i.e. WC) */
 	sbuf = kmalloc(src_len, GFP_KERNEL);
 	if (!sbuf)
@@ -199,14 +200,14 @@ void drm_fb_xrgb8888_to_rgb332(void *dst, void *src, struct drm_framebuffer *fb,
 		memcpy(sbuf, src, src_len);
 		drm_fb_xrgb8888_to_rgb332_line(dst, sbuf, width);
 		src += fb->pitches[0];
-		dst += width;
+		dst += dst_pitch;
 	}
 
 	kfree(sbuf);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
 
-static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf,
+static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, const u32 *sbuf,
 					   unsigned int pixels,
 					   bool swab)
 {
@@ -227,6 +228,7 @@ static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf,
 /**
  * drm_fb_xrgb8888_to_rgb565 - Convert XRGB8888 to RGB565 clip buffer
  * @dst: RGB565 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -234,13 +236,10 @@ static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf,
  *
  * Drivers can use this function for RGB565 devices that don't natively
  * support XRGB8888.
- *
- * This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
  */
-void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
-			       struct drm_framebuffer *fb,
-			       struct drm_rect *clip, bool swab)
+void drm_fb_xrgb8888_to_rgb565(void *dst, unsigned int dst_pitch, const void *vaddr,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip,
+			       bool swab)
 {
 	size_t linepixels = clip->x2 - clip->x1;
 	size_t src_len = linepixels * sizeof(u32);
@@ -248,6 +247,9 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
 	unsigned y, lines = clip->y2 - clip->y1;
 	void *sbuf;
 
+	if (!dst_pitch)
+		dst_pitch = dst_len;
+
 	/*
 	 * The cma memory is write-combined so reads are uncached.
 	 * Speed up by fetching one line at a time.
@@ -261,7 +263,7 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
 		memcpy(sbuf, vaddr, src_len);
 		drm_fb_xrgb8888_to_rgb565_line(dst, sbuf, linepixels, swab);
 		vaddr += fb->pitches[0];
-		dst += dst_len;
+		dst += dst_pitch;
 	}
 
 	kfree(sbuf);
@@ -269,9 +271,9 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
 
 /**
- * drm_fb_xrgb8888_to_rgb565_dstclip - Convert XRGB8888 to RGB565 clip buffer
+ * drm_fb_xrgb8888_to_rgb565_toio - Convert XRGB8888 to RGB565 clip buffer
  * @dst: RGB565 destination buffer (iomem)
- * @dst_pitch: destination buffer pitch
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -279,37 +281,36 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
  *
  * Drivers can use this function for RGB565 devices that don't natively
  * support XRGB8888.
- *
- * This function applies clipping on dst, i.e. the destination is a
- * full (iomem) framebuffer but only the clip rect content is copied over.
  */
-void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst, unsigned int dst_pitch,
-				       void *vaddr, struct drm_framebuffer *fb,
-				       struct drm_rect *clip, bool swab)
+void drm_fb_xrgb8888_to_rgb565_toio(void __iomem *dst, unsigned int dst_pitch,
+				    const void *vaddr, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip, bool swab)
 {
 	size_t linepixels = clip->x2 - clip->x1;
 	size_t dst_len = linepixels * sizeof(u16);
 	unsigned y, lines = clip->y2 - clip->y1;
 	void *dbuf;
 
+	if (!dst_pitch)
+		dst_pitch = dst_len;
+
 	dbuf = kmalloc(dst_len, GFP_KERNEL);
 	if (!dbuf)
 		return;
 
 	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
-	dst += clip_offset(clip, dst_pitch, sizeof(u16));
 	for (y = 0; y < lines; y++) {
 		drm_fb_xrgb8888_to_rgb565_line(dbuf, vaddr, linepixels, swab);
 		memcpy_toio(dst, dbuf, dst_len);
 		vaddr += fb->pitches[0];
-		dst += dst_len;
+		dst += dst_pitch;
 	}
 
 	kfree(dbuf);
 }
-EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_dstclip);
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_toio);
 
-static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, u32 *sbuf,
+static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, const u32 *sbuf,
 					   unsigned int pixels)
 {
 	unsigned int x;
@@ -324,24 +325,25 @@ static void drm_fb_xrgb8888_to_rgb888_line(u8 *dbuf, u32 *sbuf,
 /**
  * drm_fb_xrgb8888_to_rgb888 - Convert XRGB8888 to RGB888 clip buffer
  * @dst: RGB888 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @src: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * Drivers can use this function for RGB888 devices that don't natively
  * support XRGB8888.
- *
- * This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
  */
-void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
-			       struct drm_rect *clip)
+void drm_fb_xrgb8888_to_rgb888(void *dst, unsigned int dst_pitch, const void *src,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	size_t width = drm_rect_width(clip);
 	size_t src_len = width * sizeof(u32);
 	unsigned int y;
 	void *sbuf;
 
+	if (!dst_pitch)
+		dst_pitch = width * 3;
+
 	/* Use a buffer to speed up access on buffers with uncached read mapping (i.e. WC) */
 	sbuf = kmalloc(src_len, GFP_KERNEL);
 	if (!sbuf)
@@ -352,7 +354,7 @@ void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
 		memcpy(sbuf, src, src_len);
 		drm_fb_xrgb8888_to_rgb888_line(dst, sbuf, width);
 		src += fb->pitches[0];
-		dst += width * 3;
+		dst += dst_pitch;
 	}
 
 	kfree(sbuf);
@@ -360,48 +362,48 @@ void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
 
 /**
- * drm_fb_xrgb8888_to_rgb888_dstclip - Convert XRGB8888 to RGB888 clip buffer
+ * drm_fb_xrgb8888_to_rgb888_toio - Convert XRGB8888 to RGB888 clip buffer
  * @dst: RGB565 destination buffer (iomem)
- * @dst_pitch: destination buffer pitch
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * Drivers can use this function for RGB888 devices that don't natively
  * support XRGB8888.
- *
- * This function applies clipping on dst, i.e. the destination is a
- * full (iomem) framebuffer but only the clip rect content is copied over.
  */
-void drm_fb_xrgb8888_to_rgb888_dstclip(void __iomem *dst, unsigned int dst_pitch,
-				       void *vaddr, struct drm_framebuffer *fb,
-				       struct drm_rect *clip)
+void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch,
+				    const void *vaddr, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip)
 {
 	size_t linepixels = clip->x2 - clip->x1;
 	size_t dst_len = linepixels * 3;
 	unsigned y, lines = clip->y2 - clip->y1;
 	void *dbuf;
 
+	if (!dst_pitch)
+		dst_pitch = dst_len;
+
 	dbuf = kmalloc(dst_len, GFP_KERNEL);
 	if (!dbuf)
 		return;
 
 	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
-	dst += clip_offset(clip, dst_pitch, sizeof(u16));
 	for (y = 0; y < lines; y++) {
 		drm_fb_xrgb8888_to_rgb888_line(dbuf, vaddr, linepixels);
 		memcpy_toio(dst, dbuf, dst_len);
 		vaddr += fb->pitches[0];
-		dst += dst_len;
+		dst += dst_pitch;
 	}
 
 	kfree(dbuf);
 }
-EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_toio);
 
 /**
  * drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
  * @dst: 8-bit grayscale destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
  *
  * ITU BT.601 is used for the RGB -> luma (brightness) conversion.
  */
-void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
-			       struct drm_rect *clip)
+void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
+			      const struct drm_framebuffer *fb, const struct drm_rect *clip)
 {
 	unsigned int len = (clip->x2 - clip->x1) * sizeof(u32);
 	unsigned int x, y;
 	void *buf;
-	u32 *src;
+	u8 *dst8;
+	u32 *src32;
 
 	if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
 		return;
+
+	if (!dst_pitch)
+		dst_pitch = len;
+
 	/*
 	 * The cma memory is write-combined so reads are uncached.
 	 * Speed up by fetching one line at a time.
@@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
 	if (!buf)
 		return;
 
+	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
 	for (y = clip->y1; y < clip->y2; y++) {
-		src = vaddr + (y * fb->pitches[0]);
-		src += clip->x1;
-		memcpy(buf, src, len);
-		src = buf;
+		dst8 = dst;
+		src32 = memcpy(buf, vaddr, len);
 		for (x = clip->x1; x < clip->x2; x++) {
-			u8 r = (*src & 0x00ff0000) >> 16;
-			u8 g = (*src & 0x0000ff00) >> 8;
-			u8 b =  *src & 0x000000ff;
+			u8 r = (*src32 & 0x00ff0000) >> 16;
+			u8 g = (*src32 & 0x0000ff00) >> 8;
+			u8 b =  *src32 & 0x000000ff;
 
 			/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
-			*dst++ = (3 * r + 6 * g + b) / 10;
-			src++;
+			*dst8++ = (3 * r + 6 * g + b) / 10;
+			src32++;
 		}
+
+		vaddr += fb->pitches[0];
+		dst += dst_pitch;
 	}
 
 	kfree(buf);
@@ -493,15 +502,15 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 
 	} else if (dst_format == DRM_FORMAT_RGB565) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
-			drm_fb_xrgb8888_to_rgb565_dstclip(dst, dst_pitch,
-							  vmap, fb, clip,
-							  false);
+			dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
+			drm_fb_xrgb8888_to_rgb565_toio(dst, dst_pitch, vmap, fb, clip,
+						       false);
 			return 0;
 		}
 	} else if (dst_format == DRM_FORMAT_RGB888) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
-			drm_fb_xrgb8888_to_rgb888_dstclip(dst, dst_pitch,
-							  vmap, fb, clip);
+			dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
+			drm_fb_xrgb8888_to_rgb888_toio(dst, dst_pitch, vmap, fb, clip);
 			return 0;
 		}
 	}
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 7ce89917d761..b75403f3251a 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -216,7 +216,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 			drm_fb_memcpy(dst, 0, src, fb, clip);
 		break;
 	case DRM_FORMAT_XRGB8888:
-		drm_fb_xrgb8888_to_rgb565(dst, src, fb, clip, swap);
+		drm_fb_xrgb8888_to_rgb565(dst, 0, src, fb, clip, swap);
 		break;
 	default:
 		drm_err_once(fb->dev, "Format is not supported: %p4cc\n",
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index e0b117b2559f..a150a5a4b5d4 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -74,7 +74,7 @@ static size_t gud_xrgb8888_to_r124(u8 *dst, const struct drm_format_info *format
 	if (!buf)
 		return 0;
 
-	drm_fb_xrgb8888_to_gray8(buf, src, fb, rect);
+	drm_fb_xrgb8888_to_gray8(buf, 0, src, fb, rect);
 	pix8 = buf;
 
 	for (y = 0; y < height; y++) {
@@ -190,13 +190,13 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 				goto end_cpu_access;
 			}
 		} else if (format->format == DRM_FORMAT_R8) {
-			drm_fb_xrgb8888_to_gray8(buf, vaddr, fb, rect);
+			drm_fb_xrgb8888_to_gray8(buf, 0, vaddr, fb, rect);
 		} else if (format->format == DRM_FORMAT_RGB332) {
-			drm_fb_xrgb8888_to_rgb332(buf, vaddr, fb, rect);
+			drm_fb_xrgb8888_to_rgb332(buf, 0, vaddr, fb, rect);
 		} else if (format->format == DRM_FORMAT_RGB565) {
-			drm_fb_xrgb8888_to_rgb565(buf, vaddr, fb, rect, gud_is_big_endian());
+			drm_fb_xrgb8888_to_rgb565(buf, 0, vaddr, fb, rect, gud_is_big_endian());
 		} else if (format->format == DRM_FORMAT_RGB888) {
-			drm_fb_xrgb8888_to_rgb888(buf, vaddr, fb, rect);
+			drm_fb_xrgb8888_to_rgb888(buf, 0, vaddr, fb, rect);
 		} else {
 			len = gud_xrgb8888_to_color(buf, format, vaddr, fb, rect);
 		}
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 5344e506e8a9..9327001d35dd 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -329,14 +329,12 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, const struct dma_buf_
 		drm_fb_memcpy_toio(dst, fb->pitches[0], vmap, fb, rect);
 
 	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2) {
-		drm_fb_xrgb8888_to_rgb565_dstclip(cirrus->vram,
-						  cirrus->pitch,
-						  vmap, fb, rect, false);
+		dst += drm_fb_clip_offset(cirrus->pitch, fb->format, rect);
+		drm_fb_xrgb8888_to_rgb565_toio(dst, cirrus->pitch, vmap, fb, rect, false);
 
 	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3) {
-		drm_fb_xrgb8888_to_rgb888_dstclip(cirrus->vram,
-						  cirrus->pitch,
-						  vmap, fb, rect);
+		dst += drm_fb_clip_offset(cirrus->pitch, fb->format, rect);
+		drm_fb_xrgb8888_to_rgb888_toio(dst, cirrus->pitch, vmap, fb, rect);
 
 	} else {
 		WARN_ON_ONCE("cpp mismatch");
diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index 4d07b21a16e6..97a775c48cea 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -560,7 +560,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
 	if (ret)
 		goto out_free;
 
-	drm_fb_xrgb8888_to_gray8(buf, cma_obj->vaddr, fb, &clip);
+	drm_fb_xrgb8888_to_gray8(buf, 0, cma_obj->vaddr, fb, &clip);
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index ad0faa8723c2..51b9b9fb3ead 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -73,7 +73,7 @@ static void st7586_xrgb8888_to_gray332(u8 *dst, void *vaddr,
 	if (!buf)
 		return;
 
-	drm_fb_xrgb8888_to_gray8(buf, vaddr, fb, clip);
+	drm_fb_xrgb8888_to_gray8(buf, 0, vaddr, fb, clip);
 	src = buf;
 
 	for (y = clip->y1; y < clip->y2; y++) {
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index c4c3c845e119..8af9b8f35dd6 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -20,21 +20,21 @@ void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void *v
 void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
 		 const struct drm_framebuffer *fb, const struct drm_rect *clip,
 		 bool cached);
-void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb,
-			       struct drm_rect *clip);
-void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
-			       struct drm_framebuffer *fb,
-			       struct drm_rect *clip, bool swab);
-void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst, unsigned int dst_pitch,
-				       void *vaddr, struct drm_framebuffer *fb,
-				       struct drm_rect *clip, bool swab);
-void drm_fb_xrgb8888_to_rgb888(void *dst, void *src, struct drm_framebuffer *fb,
-			       struct drm_rect *clip);
-void drm_fb_xrgb8888_to_rgb888_dstclip(void __iomem *dst, unsigned int dst_pitch,
-				       void *vaddr, struct drm_framebuffer *fb,
-				       struct drm_rect *clip);
-void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
-			      struct drm_rect *clip);
+void drm_fb_xrgb8888_to_rgb332(void *dst, unsigned int dst_pitch, const void *vaddr,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_rgb565(void *dst, unsigned int dst_pitch, const void *vaddr,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip,
+			       bool swab);
+void drm_fb_xrgb8888_to_rgb565_toio(void __iomem *dst, unsigned int dst_pitch,
+				    const void *vaddr, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip, bool swab);
+void drm_fb_xrgb8888_to_rgb888(void *dst, unsigned int dst_pitch, const void *src,
+			       const struct drm_framebuffer *fb, const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch,
+				    const void *vaddr, const struct drm_framebuffer *fb,
+				    const struct drm_rect *clip);
+void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
+			      const struct drm_framebuffer *fb, const struct drm_rect *clip);
 
 int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 			     uint32_t dst_format, void *vmap,
-- 
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 4/9] drm/format-helper: Rework format-helper conversion functions

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Move destination-buffer clipping from all format-helper conversion
> functions into callers. Support destination-buffer pitch. Only
> distinguish between system and I/O memory, but use same logic
> everywhere.
> 
> Simply harmonize the interface and semantics of the existing code.
> Not all conversion helpers support all combinations of parameters.
> We have to add additional features when we need them.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

>  /**
>   * drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
>   * @dst: 8-bit grayscale destination buffer
> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
>   * @vaddr: XRGB8888 source buffer
>   * @fb: DRM framebuffer
>   * @clip: Clip rectangle area to copy
> @@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
>   *
>   * ITU BT.601 is used for the RGB -> luma (brightness) conversion.
>   */
> -void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
> -			       struct drm_rect *clip)
> +void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
> +			      const struct drm_framebuffer *fb, const struct drm_rect *clip)
>  {
>  	unsigned int len = (clip->x2 - clip->x1) * sizeof(u32);
>  	unsigned int x, y;
>  	void *buf;
> -	u32 *src;
> +	u8 *dst8;
> +	u32 *src32;
>  
>  	if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
>  		return;
> +
> +	if (!dst_pitch)

len is source length (should really have been named src_len) which
results in a kernel crash:

> +		dst_pitch = len;

This works:

		dst_pitch = drm_rect_width(clip);

With that fixed:

Tested-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

> +
>  	/*
>  	 * The cma memory is write-combined so reads are uncached.
>  	 * Speed up by fetching one line at a time.
> @@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>  	if (!buf)
>  		return;
>  
> +	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
>  	for (y = clip->y1; y < clip->y2; y++) {
> -		src = vaddr + (y * fb->pitches[0]);
> -		src += clip->x1;
> -		memcpy(buf, src, len);
> -		src = buf;
> +		dst8 = dst;
> +		src32 = memcpy(buf, vaddr, len);
>  		for (x = clip->x1; x < clip->x2; x++) {
> -			u8 r = (*src & 0x00ff0000) >> 16;
> -			u8 g = (*src & 0x0000ff00) >> 8;
> -			u8 b =  *src & 0x000000ff;
> +			u8 r = (*src32 & 0x00ff0000) >> 16;
> +			u8 g = (*src32 & 0x0000ff00) >> 8;
> +			u8 b =  *src32 & 0x000000ff;
>  
>  			/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
> -			*dst++ = (3 * r + 6 * g + b) / 10;
> -			src++;
> +			*dst8++ = (3 * r + 6 * g + b) / 10;
> +			src32++;
>  		}
> +
> +		vaddr += fb->pitches[0];
> +		dst += dst_pitch;
>  	}
>  
>  	kfree(buf);

Re: [PATCH 4/9] drm/format-helper: Rework format-helper conversion functions

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Move destination-buffer clipping from all format-helper conversion
> functions into callers. Support destination-buffer pitch. Only
> distinguish between system and I/O memory, but use same logic
> everywhere.
> 
> Simply harmonize the interface and semantics of the existing code.
> Not all conversion helpers support all combinations of parameters.
> We have to add additional features when we need them.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

>  /**
>   * drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
>   * @dst: 8-bit grayscale destination buffer
> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
>   * @vaddr: XRGB8888 source buffer
>   * @fb: DRM framebuffer
>   * @clip: Clip rectangle area to copy
> @@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
>   *
>   * ITU BT.601 is used for the RGB -> luma (brightness) conversion.
>   */
> -void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
> -			       struct drm_rect *clip)
> +void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
> +			      const struct drm_framebuffer *fb, const struct drm_rect *clip)
>  {
>  	unsigned int len = (clip->x2 - clip->x1) * sizeof(u32);
>  	unsigned int x, y;
>  	void *buf;
> -	u32 *src;
> +	u8 *dst8;
> +	u32 *src32;
>  
>  	if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
>  		return;
> +
> +	if (!dst_pitch)

len is source length (should really have been named src_len) which
results in a kernel crash:

> +		dst_pitch = len;

This works:

		dst_pitch = drm_rect_width(clip);

With that fixed:

Tested-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

> +
>  	/*
>  	 * The cma memory is write-combined so reads are uncached.
>  	 * Speed up by fetching one line at a time.
> @@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>  	if (!buf)
>  		return;
>  
> +	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
>  	for (y = clip->y1; y < clip->y2; y++) {
> -		src = vaddr + (y * fb->pitches[0]);
> -		src += clip->x1;
> -		memcpy(buf, src, len);
> -		src = buf;
> +		dst8 = dst;
> +		src32 = memcpy(buf, vaddr, len);
>  		for (x = clip->x1; x < clip->x2; x++) {
> -			u8 r = (*src & 0x00ff0000) >> 16;
> -			u8 g = (*src & 0x0000ff00) >> 8;
> -			u8 b =  *src & 0x000000ff;
> +			u8 r = (*src32 & 0x00ff0000) >> 16;
> +			u8 g = (*src32 & 0x0000ff00) >> 8;
> +			u8 b =  *src32 & 0x000000ff;
>  
>  			/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
> -			*dst++ = (3 * r + 6 * g + b) / 10;
> -			src++;
> +			*dst8++ = (3 * r + 6 * g + b) / 10;
> +			src32++;
>  		}
> +
> +		vaddr += fb->pitches[0];
> +		dst += dst_pitch;
>  	}
>  
>  	kfree(buf);
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 4/9] drm/format-helper: Rework format-helper conversion functions

[Thomas Zimmermann]

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

Hi

Am 24.10.21 um 13:32 schrieb Noralf Trønnes:
> 
> 
> Den 22.10.2021 15.28, skrev Thomas Zimmermann:
>> Move destination-buffer clipping from all format-helper conversion
>> functions into callers. Support destination-buffer pitch. Only
>> distinguish between system and I/O memory, but use same logic
>> everywhere.
>>
>> Simply harmonize the interface and semantics of the existing code.
>> Not all conversion helpers support all combinations of parameters.
>> We have to add additional features when we need them.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
> 
>>   /**
>>    * drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
>>    * @dst: 8-bit grayscale destination buffer
>> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
>>    * @vaddr: XRGB8888 source buffer
>>    * @fb: DRM framebuffer
>>    * @clip: Clip rectangle area to copy
>> @@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
>>    *
>>    * ITU BT.601 is used for the RGB -> luma (brightness) conversion.
>>    */
>> -void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>> -			       struct drm_rect *clip)
>> +void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
>> +			      const struct drm_framebuffer *fb, const struct drm_rect *clip)
>>   {
>>   	unsigned int len = (clip->x2 - clip->x1) * sizeof(u32);
>>   	unsigned int x, y;
>>   	void *buf;
>> -	u32 *src;
>> +	u8 *dst8;
>> +	u32 *src32;
>>   
>>   	if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
>>   		return;
>> +
>> +	if (!dst_pitch)
> 
> len is source length (should really have been named src_len) which
> results in a kernel crash:
> 
>> +		dst_pitch = len;
> 
> This works:
> 
> 		dst_pitch = drm_rect_width(clip);

Fixed in the next revision. Thank you so much!

Best regards
Thomas

> 
> With that fixed:
> 
> Tested-by: Noralf Trønnes <noralf@tronnes.org>
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
> 
>> +
>>   	/*
>>   	 * The cma memory is write-combined so reads are uncached.
>>   	 * Speed up by fetching one line at a time.
>> @@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>>   	if (!buf)
>>   		return;
>>   
>> +	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
>>   	for (y = clip->y1; y < clip->y2; y++) {
>> -		src = vaddr + (y * fb->pitches[0]);
>> -		src += clip->x1;
>> -		memcpy(buf, src, len);
>> -		src = buf;
>> +		dst8 = dst;
>> +		src32 = memcpy(buf, vaddr, len);
>>   		for (x = clip->x1; x < clip->x2; x++) {
>> -			u8 r = (*src & 0x00ff0000) >> 16;
>> -			u8 g = (*src & 0x0000ff00) >> 8;
>> -			u8 b =  *src & 0x000000ff;
>> +			u8 r = (*src32 & 0x00ff0000) >> 16;
>> +			u8 g = (*src32 & 0x0000ff00) >> 8;
>> +			u8 b =  *src32 & 0x000000ff;
>>   
>>   			/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
>> -			*dst++ = (3 * r + 6 * g + b) / 10;
>> -			src++;
>> +			*dst8++ = (3 * r + 6 * g + b) / 10;
>> +			src32++;
>>   		}
>> +
>> +		vaddr += fb->pitches[0];
>> +		dst += dst_pitch;
>>   	}
>>   
>>   	kfree(buf);

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

Re: [PATCH 4/9] drm/format-helper: Rework format-helper conversion functions

[Thomas Zimmermann]

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

Hi

Am 24.10.21 um 13:32 schrieb Noralf Trønnes:
> 
> 
> Den 22.10.2021 15.28, skrev Thomas Zimmermann:
>> Move destination-buffer clipping from all format-helper conversion
>> functions into callers. Support destination-buffer pitch. Only
>> distinguish between system and I/O memory, but use same logic
>> everywhere.
>>
>> Simply harmonize the interface and semantics of the existing code.
>> Not all conversion helpers support all combinations of parameters.
>> We have to add additional features when we need them.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
> 
>>   /**
>>    * drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
>>    * @dst: 8-bit grayscale destination buffer
>> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
>>    * @vaddr: XRGB8888 source buffer
>>    * @fb: DRM framebuffer
>>    * @clip: Clip rectangle area to copy
>> @@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
>>    *
>>    * ITU BT.601 is used for the RGB -> luma (brightness) conversion.
>>    */
>> -void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>> -			       struct drm_rect *clip)
>> +void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
>> +			      const struct drm_framebuffer *fb, const struct drm_rect *clip)
>>   {
>>   	unsigned int len = (clip->x2 - clip->x1) * sizeof(u32);
>>   	unsigned int x, y;
>>   	void *buf;
>> -	u32 *src;
>> +	u8 *dst8;
>> +	u32 *src32;
>>   
>>   	if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
>>   		return;
>> +
>> +	if (!dst_pitch)
> 
> len is source length (should really have been named src_len) which
> results in a kernel crash:
> 
>> +		dst_pitch = len;
> 
> This works:
> 
> 		dst_pitch = drm_rect_width(clip);

Fixed in the next revision. Thank you so much!

Best regards
Thomas

> 
> With that fixed:
> 
> Tested-by: Noralf Trønnes <noralf@tronnes.org>
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
> 
>> +
>>   	/*
>>   	 * The cma memory is write-combined so reads are uncached.
>>   	 * Speed up by fetching one line at a time.
>> @@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>>   	if (!buf)
>>   		return;
>>   
>> +	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
>>   	for (y = clip->y1; y < clip->y2; y++) {
>> -		src = vaddr + (y * fb->pitches[0]);
>> -		src += clip->x1;
>> -		memcpy(buf, src, len);
>> -		src = buf;
>> +		dst8 = dst;
>> +		src32 = memcpy(buf, vaddr, len);
>>   		for (x = clip->x1; x < clip->x2; x++) {
>> -			u8 r = (*src & 0x00ff0000) >> 16;
>> -			u8 g = (*src & 0x0000ff00) >> 8;
>> -			u8 b =  *src & 0x000000ff;
>> +			u8 r = (*src32 & 0x00ff0000) >> 16;
>> +			u8 g = (*src32 & 0x0000ff00) >> 8;
>> +			u8 b =  *src32 & 0x000000ff;
>>   
>>   			/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
>> -			*dst++ = (3 * r + 6 * g + b) / 10;
>> -			src++;
>> +			*dst8++ = (3 * r + 6 * g + b) / 10;
>> +			src32++;
>>   		}
>> +
>> +		vaddr += fb->pitches[0];
>> +		dst += dst_pitch;
>>   	}
>>   
>>   	kfree(buf);

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

Re: [PATCH 4/9] drm/format-helper: Rework format-helper conversion functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3408 bytes --]

Hi

Am 24.10.21 um 13:32 schrieb Noralf Trønnes:
> 
> 
> Den 22.10.2021 15.28, skrev Thomas Zimmermann:
>> Move destination-buffer clipping from all format-helper conversion
>> functions into callers. Support destination-buffer pitch. Only
>> distinguish between system and I/O memory, but use same logic
>> everywhere.
>>
>> Simply harmonize the interface and semantics of the existing code.
>> Not all conversion helpers support all combinations of parameters.
>> We have to add additional features when we need them.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
> 
>>   /**
>>    * drm_fb_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale
>>    * @dst: 8-bit grayscale destination buffer
>> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
>>    * @vaddr: XRGB8888 source buffer
>>    * @fb: DRM framebuffer
>>    * @clip: Clip rectangle area to copy
>> @@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
>>    *
>>    * ITU BT.601 is used for the RGB -> luma (brightness) conversion.
>>    */
>> -void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>> -			       struct drm_rect *clip)
>> +void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
>> +			      const struct drm_framebuffer *fb, const struct drm_rect *clip)
>>   {
>>   	unsigned int len = (clip->x2 - clip->x1) * sizeof(u32);
>>   	unsigned int x, y;
>>   	void *buf;
>> -	u32 *src;
>> +	u8 *dst8;
>> +	u32 *src32;
>>   
>>   	if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB8888))
>>   		return;
>> +
>> +	if (!dst_pitch)
> 
> len is source length (should really have been named src_len) which
> results in a kernel crash:
> 
>> +		dst_pitch = len;
> 
> This works:
> 
> 		dst_pitch = drm_rect_width(clip);

Fixed in the next revision. Thank you so much!

Best regards
Thomas

> 
> With that fixed:
> 
> Tested-by: Noralf Trønnes <noralf@tronnes.org>
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
> 
>> +
>>   	/*
>>   	 * The cma memory is write-combined so reads are uncached.
>>   	 * Speed up by fetching one line at a time.
>> @@ -433,20 +440,22 @@ void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>>   	if (!buf)
>>   		return;
>>   
>> +	vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
>>   	for (y = clip->y1; y < clip->y2; y++) {
>> -		src = vaddr + (y * fb->pitches[0]);
>> -		src += clip->x1;
>> -		memcpy(buf, src, len);
>> -		src = buf;
>> +		dst8 = dst;
>> +		src32 = memcpy(buf, vaddr, len);
>>   		for (x = clip->x1; x < clip->x2; x++) {
>> -			u8 r = (*src & 0x00ff0000) >> 16;
>> -			u8 g = (*src & 0x0000ff00) >> 8;
>> -			u8 b =  *src & 0x000000ff;
>> +			u8 r = (*src32 & 0x00ff0000) >> 16;
>> +			u8 g = (*src32 & 0x0000ff00) >> 8;
>> +			u8 b =  *src32 & 0x000000ff;
>>   
>>   			/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
>> -			*dst++ = (3 * r + 6 * g + b) / 10;
>> -			src++;
>> +			*dst8++ = (3 * r + 6 * g + b) / 10;
>> +			src32++;
>>   		}
>> +
>> +		vaddr += fb->pitches[0];
>> +		dst += dst_pitch;
>>   	}
>>   
>>   	kfree(buf);

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 5/9] drm/format-helper: Streamline blit-helper interface

[Thomas Zimmermann]

Move destination-buffer clipping from format-helper blit function into
caller. Rename drm_fb_blit_rect_dstclip() to drm_fb_blit_toio(). Done for
consistency with the rest of the interface. Remove drm_fb_blit_dstclip(),
which isn't required.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 51 ++++-------------------------
 drivers/gpu/drm/tiny/simpledrm.c    | 14 +++++---
 include/drm/drm_format_helper.h     | 10 ++----
 3 files changed, 19 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 260dc587c1d7..5ca9abc65510 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -463,7 +463,7 @@ void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vad
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
 
 /**
- * drm_fb_blit_rect_dstclip - Copy parts of a framebuffer to display memory
+ * drm_fb_blit_toio - Copy parts of a framebuffer to display memory
  * @dst:	The display memory to copy to
  * @dst_pitch:	Number of bytes between two consecutive scanlines within dst
  * @dst_format:	FOURCC code of the display's color format
@@ -475,17 +475,14 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
  * formats of the display and the framebuffer mismatch, the blit function
  * will attempt to convert between them.
  *
- * Use drm_fb_blit_dstclip() to copy the full framebuffer.
- *
  * Returns:
  * 0 on success, or
  * -EINVAL if the color-format conversion failed, or
  * a negative error code otherwise.
  */
-int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			     uint32_t dst_format, void *vmap,
-			     struct drm_framebuffer *fb,
-			     struct drm_rect *clip)
+int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_format,
+		     const void *vmap, const struct drm_framebuffer *fb,
+		     const struct drm_rect *clip)
 {
 	uint32_t fb_format = fb->format->format;
 
@@ -496,20 +493,16 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 		dst_format = DRM_FORMAT_XRGB8888;
 
 	if (dst_format == fb_format) {
-		dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
 		drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip);
 		return 0;
 
 	} else if (dst_format == DRM_FORMAT_RGB565) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
-			dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
-			drm_fb_xrgb8888_to_rgb565_toio(dst, dst_pitch, vmap, fb, clip,
-						       false);
+			drm_fb_xrgb8888_to_rgb565_toio(dst, dst_pitch, vmap, fb, clip, false);
 			return 0;
 		}
 	} else if (dst_format == DRM_FORMAT_RGB888) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
-			dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
 			drm_fb_xrgb8888_to_rgb888_toio(dst, dst_pitch, vmap, fb, clip);
 			return 0;
 		}
@@ -517,36 +510,4 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 
 	return -EINVAL;
 }
-EXPORT_SYMBOL(drm_fb_blit_rect_dstclip);
-
-/**
- * drm_fb_blit_dstclip - Copy framebuffer to display memory
- * @dst:	The display memory to copy to
- * @dst_pitch:	Number of bytes between two consecutive scanlines within dst
- * @dst_format:	FOURCC code of the display's color format
- * @vmap:	The framebuffer memory to copy from
- * @fb:		The framebuffer to copy from
- *
- * This function copies a full framebuffer to display memory. If the formats
- * of the display and the framebuffer mismatch, the copy function will
- * attempt to convert between them.
- *
- * See drm_fb_blit_rect_dstclip() for more information.
- *
- * Returns:
- * 0 on success, or a negative error code otherwise.
- */
-int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			uint32_t dst_format, void *vmap,
-			struct drm_framebuffer *fb)
-{
-	struct drm_rect fullscreen = {
-		.x1 = 0,
-		.x2 = fb->width,
-		.y1 = 0,
-		.y2 = fb->height,
-	};
-	return drm_fb_blit_rect_dstclip(dst, dst_pitch, dst_format, vmap, fb,
-					&fullscreen);
-}
-EXPORT_SYMBOL(drm_fb_blit_dstclip);
+EXPORT_SYMBOL(drm_fb_blit_toio);
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 481b48bde047..571f716ff427 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -641,6 +641,8 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	struct drm_framebuffer *fb = plane_state->fb;
 	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
 	struct drm_device *dev = &sdev->dev;
+	void __iomem *dst = sdev->screen_base;
+	struct drm_rect clip;
 	int idx;
 
 	if (!fb)
@@ -649,8 +651,11 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (!drm_dev_enter(dev, &idx))
 		return;
 
-	drm_fb_blit_dstclip(sdev->screen_base, sdev->pitch,
-			    sdev->format->format, vmap, fb);
+	drm_rect_init(&clip, 0, 0, fb->width, fb->height);
+
+	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
+	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
+
 	drm_dev_exit(idx);
 }
 
@@ -680,6 +685,7 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
 	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_device *dev = &sdev->dev;
+	void __iomem *dst = sdev->screen_base;
 	struct drm_rect clip;
 	int idx;
 
@@ -692,8 +698,8 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
 	if (!drm_dev_enter(dev, &idx))
 		return;
 
-	drm_fb_blit_rect_dstclip(sdev->screen_base, sdev->pitch,
-				 sdev->format->format, vmap, fb, &clip);
+	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
+	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
 
 	drm_dev_exit(idx);
 }
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 8af9b8f35dd6..c91b7c553878 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -36,12 +36,8 @@ void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch,
 void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
 			      const struct drm_framebuffer *fb, const struct drm_rect *clip);
 
-int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			     uint32_t dst_format, void *vmap,
-			     struct drm_framebuffer *fb,
-			     struct drm_rect *rect);
-int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			uint32_t dst_format, void *vmap,
-			struct drm_framebuffer *fb);
+int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_format,
+		     const void *vmap, const struct drm_framebuffer *fb,
+		     const struct drm_rect *rect);
 
 #endif /* __LINUX_DRM_FORMAT_HELPER_H */
-- 
2.33.0

[PATCH 5/9] drm/format-helper: Streamline blit-helper interface

[Thomas Zimmermann]

Move destination-buffer clipping from format-helper blit function into
caller. Rename drm_fb_blit_rect_dstclip() to drm_fb_blit_toio(). Done for
consistency with the rest of the interface. Remove drm_fb_blit_dstclip(),
which isn't required.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 51 ++++-------------------------
 drivers/gpu/drm/tiny/simpledrm.c    | 14 +++++---
 include/drm/drm_format_helper.h     | 10 ++----
 3 files changed, 19 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 260dc587c1d7..5ca9abc65510 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -463,7 +463,7 @@ void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vad
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
 
 /**
- * drm_fb_blit_rect_dstclip - Copy parts of a framebuffer to display memory
+ * drm_fb_blit_toio - Copy parts of a framebuffer to display memory
  * @dst:	The display memory to copy to
  * @dst_pitch:	Number of bytes between two consecutive scanlines within dst
  * @dst_format:	FOURCC code of the display's color format
@@ -475,17 +475,14 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
  * formats of the display and the framebuffer mismatch, the blit function
  * will attempt to convert between them.
  *
- * Use drm_fb_blit_dstclip() to copy the full framebuffer.
- *
  * Returns:
  * 0 on success, or
  * -EINVAL if the color-format conversion failed, or
  * a negative error code otherwise.
  */
-int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			     uint32_t dst_format, void *vmap,
-			     struct drm_framebuffer *fb,
-			     struct drm_rect *clip)
+int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_format,
+		     const void *vmap, const struct drm_framebuffer *fb,
+		     const struct drm_rect *clip)
 {
 	uint32_t fb_format = fb->format->format;
 
@@ -496,20 +493,16 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 		dst_format = DRM_FORMAT_XRGB8888;
 
 	if (dst_format == fb_format) {
-		dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
 		drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip);
 		return 0;
 
 	} else if (dst_format == DRM_FORMAT_RGB565) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
-			dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
-			drm_fb_xrgb8888_to_rgb565_toio(dst, dst_pitch, vmap, fb, clip,
-						       false);
+			drm_fb_xrgb8888_to_rgb565_toio(dst, dst_pitch, vmap, fb, clip, false);
 			return 0;
 		}
 	} else if (dst_format == DRM_FORMAT_RGB888) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
-			dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
 			drm_fb_xrgb8888_to_rgb888_toio(dst, dst_pitch, vmap, fb, clip);
 			return 0;
 		}
@@ -517,36 +510,4 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
 
 	return -EINVAL;
 }
-EXPORT_SYMBOL(drm_fb_blit_rect_dstclip);
-
-/**
- * drm_fb_blit_dstclip - Copy framebuffer to display memory
- * @dst:	The display memory to copy to
- * @dst_pitch:	Number of bytes between two consecutive scanlines within dst
- * @dst_format:	FOURCC code of the display's color format
- * @vmap:	The framebuffer memory to copy from
- * @fb:		The framebuffer to copy from
- *
- * This function copies a full framebuffer to display memory. If the formats
- * of the display and the framebuffer mismatch, the copy function will
- * attempt to convert between them.
- *
- * See drm_fb_blit_rect_dstclip() for more information.
- *
- * Returns:
- * 0 on success, or a negative error code otherwise.
- */
-int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			uint32_t dst_format, void *vmap,
-			struct drm_framebuffer *fb)
-{
-	struct drm_rect fullscreen = {
-		.x1 = 0,
-		.x2 = fb->width,
-		.y1 = 0,
-		.y2 = fb->height,
-	};
-	return drm_fb_blit_rect_dstclip(dst, dst_pitch, dst_format, vmap, fb,
-					&fullscreen);
-}
-EXPORT_SYMBOL(drm_fb_blit_dstclip);
+EXPORT_SYMBOL(drm_fb_blit_toio);
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 481b48bde047..571f716ff427 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -641,6 +641,8 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	struct drm_framebuffer *fb = plane_state->fb;
 	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
 	struct drm_device *dev = &sdev->dev;
+	void __iomem *dst = sdev->screen_base;
+	struct drm_rect clip;
 	int idx;
 
 	if (!fb)
@@ -649,8 +651,11 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (!drm_dev_enter(dev, &idx))
 		return;
 
-	drm_fb_blit_dstclip(sdev->screen_base, sdev->pitch,
-			    sdev->format->format, vmap, fb);
+	drm_rect_init(&clip, 0, 0, fb->width, fb->height);
+
+	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
+	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
+
 	drm_dev_exit(idx);
 }
 
@@ -680,6 +685,7 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
 	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_device *dev = &sdev->dev;
+	void __iomem *dst = sdev->screen_base;
 	struct drm_rect clip;
 	int idx;
 
@@ -692,8 +698,8 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
 	if (!drm_dev_enter(dev, &idx))
 		return;
 
-	drm_fb_blit_rect_dstclip(sdev->screen_base, sdev->pitch,
-				 sdev->format->format, vmap, fb, &clip);
+	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
+	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
 
 	drm_dev_exit(idx);
 }
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 8af9b8f35dd6..c91b7c553878 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -36,12 +36,8 @@ void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch,
 void drm_fb_xrgb8888_to_gray8(void *dst, unsigned int dst_pitch, const void *vaddr,
 			      const struct drm_framebuffer *fb, const struct drm_rect *clip);
 
-int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			     uint32_t dst_format, void *vmap,
-			     struct drm_framebuffer *fb,
-			     struct drm_rect *rect);
-int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch,
-			uint32_t dst_format, void *vmap,
-			struct drm_framebuffer *fb);
+int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_format,
+		     const void *vmap, const struct drm_framebuffer *fb,
+		     const struct drm_rect *rect);
 
 #endif /* __LINUX_DRM_FORMAT_HELPER_H */
-- 
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 5/9] drm/format-helper: Streamline blit-helper interface

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Move destination-buffer clipping from format-helper blit function into
> caller. Rename drm_fb_blit_rect_dstclip() to drm_fb_blit_toio(). Done for
> consistency with the rest of the interface. Remove drm_fb_blit_dstclip(),
> which isn't required.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

Re: [PATCH 5/9] drm/format-helper: Streamline blit-helper interface

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Move destination-buffer clipping from format-helper blit function into
> caller. Rename drm_fb_blit_rect_dstclip() to drm_fb_blit_toio(). Done for
> consistency with the rest of the interface. Remove drm_fb_blit_dstclip(),
> which isn't required.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 6/9] drm/fb-helper: Allocate shadow buffer of surface height

[Thomas Zimmermann]

Allocating a shadow buffer of the height of the buffer object does
not support fbdev overallocation. Use surface height instead.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_fb_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8e7a124d6c5a..9727a59d35fd 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2338,7 +2338,7 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
 		return PTR_ERR(fbi);
 
 	fbi->fbops = &drm_fbdev_fb_ops;
-	fbi->screen_size = fb->height * fb->pitches[0];
+	fbi->screen_size = sizes->surface_height * fb->pitches[0];
 	fbi->fix.smem_len = fbi->screen_size;
 
 	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
-- 
2.33.0

[PATCH 6/9] drm/fb-helper: Allocate shadow buffer of surface height

[Thomas Zimmermann]

Allocating a shadow buffer of the height of the buffer object does
not support fbdev overallocation. Use surface height instead.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_fb_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8e7a124d6c5a..9727a59d35fd 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2338,7 +2338,7 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
 		return PTR_ERR(fbi);
 
 	fbi->fbops = &drm_fbdev_fb_ops;
-	fbi->screen_size = fb->height * fb->pitches[0];
+	fbi->screen_size = sizes->surface_height * fb->pitches[0];
 	fbi->fix.smem_len = fbi->screen_size;
 
 	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
-- 
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 6/9] drm/fb-helper: Allocate shadow buffer of surface height

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Allocating a shadow buffer of the height of the buffer object does
> not support fbdev overallocation. Use surface height instead.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

Re: [PATCH 6/9] drm/fb-helper: Allocate shadow buffer of surface height

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Allocating a shadow buffer of the height of the buffer object does
> not support fbdev overallocation. Use surface height instead.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property

[Thomas Zimmermann]

Enable the FB_DAMAGE_CLIPS property to reduce display-update
overhead. Also fixes a warning in the kernel log.

  simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called

Fix the computation of the blit rectangle. This wasn't an issue so
far, as simpledrm always blitted the full framebuffer. The code now
supports damage clipping and virtual screen sizes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 571f716ff427..e872121e9fb0 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
 	struct drm_device *dev = &sdev->dev;
 	void __iomem *dst = sdev->screen_base;
-	struct drm_rect clip;
+	struct drm_rect src_clip, dst_clip;
 	int idx;
 
 	if (!fb)
@@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (!drm_dev_enter(dev, &idx))
 		return;
 
-	drm_rect_init(&clip, 0, 0, fb->width, fb->height);
+	drm_rect_fp_to_int(&src_clip, &plane_state->src);
 
-	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
-	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
+	dst_clip = plane_state->dst;
+	if (!drm_rect_intersect(&dst_clip, &src_clip))
+		return;
+
+	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
+	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
 
 	drm_dev_exit(idx);
 }
@@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_device *dev = &sdev->dev;
 	void __iomem *dst = sdev->screen_base;
-	struct drm_rect clip;
+	struct drm_rect damage_clip, src_clip, dst_clip;
 	int idx;
 
 	if (!fb)
 		return;
 
-	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip))
+	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip))
+		return;
+
+	drm_rect_fp_to_int(&src_clip, &plane_state->src);
+	if (!drm_rect_intersect(&src_clip, &damage_clip))
+		return;
+
+	dst_clip = plane_state->dst;
+	if (!drm_rect_intersect(&dst_clip, &src_clip))
 		return;
 
 	if (!drm_dev_enter(dev, &idx))
 		return;
 
-	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
-	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
+	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
+	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
 
 	drm_dev_exit(idx);
 }
@@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
 	if (ret)
 		return ret;
 
+	drm_plane_enable_fb_damage_clips(&pipe->plane);
+
 	drm_mode_config_reset(dev);
 
 	return 0;
-- 
2.33.0

[PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property

[Thomas Zimmermann]

Enable the FB_DAMAGE_CLIPS property to reduce display-update
overhead. Also fixes a warning in the kernel log.

  simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called

Fix the computation of the blit rectangle. This wasn't an issue so
far, as simpledrm always blitted the full framebuffer. The code now
supports damage clipping and virtual screen sizes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 571f716ff427..e872121e9fb0 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
 	struct drm_device *dev = &sdev->dev;
 	void __iomem *dst = sdev->screen_base;
-	struct drm_rect clip;
+	struct drm_rect src_clip, dst_clip;
 	int idx;
 
 	if (!fb)
@@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (!drm_dev_enter(dev, &idx))
 		return;
 
-	drm_rect_init(&clip, 0, 0, fb->width, fb->height);
+	drm_rect_fp_to_int(&src_clip, &plane_state->src);
 
-	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
-	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
+	dst_clip = plane_state->dst;
+	if (!drm_rect_intersect(&dst_clip, &src_clip))
+		return;
+
+	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
+	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
 
 	drm_dev_exit(idx);
 }
@@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_device *dev = &sdev->dev;
 	void __iomem *dst = sdev->screen_base;
-	struct drm_rect clip;
+	struct drm_rect damage_clip, src_clip, dst_clip;
 	int idx;
 
 	if (!fb)
 		return;
 
-	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip))
+	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip))
+		return;
+
+	drm_rect_fp_to_int(&src_clip, &plane_state->src);
+	if (!drm_rect_intersect(&src_clip, &damage_clip))
+		return;
+
+	dst_clip = plane_state->dst;
+	if (!drm_rect_intersect(&dst_clip, &src_clip))
 		return;
 
 	if (!drm_dev_enter(dev, &idx))
 		return;
 
-	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
-	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
+	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
+	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
 
 	drm_dev_exit(idx);
 }
@@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
 	if (ret)
 		return ret;
 
+	drm_plane_enable_fb_damage_clips(&pipe->plane);
+
 	drm_mode_config_reset(dev);
 
 	return 0;
-- 
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Enable the FB_DAMAGE_CLIPS property to reduce display-update
> overhead. Also fixes a warning in the kernel log.
> 
>   simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called
> 
> Fix the computation of the blit rectangle. This wasn't an issue so
> far, as simpledrm always blitted the full framebuffer. The code now
> supports damage clipping and virtual screen sizes.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 571f716ff427..e872121e9fb0 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>  	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
>  	struct drm_device *dev = &sdev->dev;
>  	void __iomem *dst = sdev->screen_base;
> -	struct drm_rect clip;
> +	struct drm_rect src_clip, dst_clip;
>  	int idx;
>  
>  	if (!fb)
> @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>  	if (!drm_dev_enter(dev, &idx))
>  		return;
>  
> -	drm_rect_init(&clip, 0, 0, fb->width, fb->height);
> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
>  
> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
> +	dst_clip = plane_state->dst;
> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
> +		return;
> +
> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>  
>  	drm_dev_exit(idx);
>  }
> @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
>  	struct drm_framebuffer *fb = plane_state->fb;
>  	struct drm_device *dev = &sdev->dev;
>  	void __iomem *dst = sdev->screen_base;
> -	struct drm_rect clip;
> +	struct drm_rect damage_clip, src_clip, dst_clip;
>  	int idx;
>  
>  	if (!fb)
>  		return;
>  
> -	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip))
> +	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip))
> +		return;
> +

I'm afraid I don't understand what's going on here, but isn't it
possible to put this logic into drm_atomic_helper_damage_merged()?

Noralf.

> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
> +	if (!drm_rect_intersect(&src_clip, &damage_clip))
> +		return;
> +
> +	dst_clip = plane_state->dst;
> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
>  		return;
>  
>  	if (!drm_dev_enter(dev, &idx))
>  		return;
>  
> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>  
>  	drm_dev_exit(idx);
>  }
> @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
>  	if (ret)
>  		return ret;
>  
> +	drm_plane_enable_fb_damage_clips(&pipe->plane);
> +
>  	drm_mode_config_reset(dev);
>  
>  	return 0;
> 

Re: [PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property

[Noralf Trønnes]

Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Enable the FB_DAMAGE_CLIPS property to reduce display-update
> overhead. Also fixes a warning in the kernel log.
> 
>   simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called
> 
> Fix the computation of the blit rectangle. This wasn't an issue so
> far, as simpledrm always blitted the full framebuffer. The code now
> supports damage clipping and virtual screen sizes.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 571f716ff427..e872121e9fb0 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>  	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
>  	struct drm_device *dev = &sdev->dev;
>  	void __iomem *dst = sdev->screen_base;
> -	struct drm_rect clip;
> +	struct drm_rect src_clip, dst_clip;
>  	int idx;
>  
>  	if (!fb)
> @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>  	if (!drm_dev_enter(dev, &idx))
>  		return;
>  
> -	drm_rect_init(&clip, 0, 0, fb->width, fb->height);
> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
>  
> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
> +	dst_clip = plane_state->dst;
> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
> +		return;
> +
> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>  
>  	drm_dev_exit(idx);
>  }
> @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
>  	struct drm_framebuffer *fb = plane_state->fb;
>  	struct drm_device *dev = &sdev->dev;
>  	void __iomem *dst = sdev->screen_base;
> -	struct drm_rect clip;
> +	struct drm_rect damage_clip, src_clip, dst_clip;
>  	int idx;
>  
>  	if (!fb)
>  		return;
>  
> -	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip))
> +	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip))
> +		return;
> +

I'm afraid I don't understand what's going on here, but isn't it
possible to put this logic into drm_atomic_helper_damage_merged()?

Noralf.

> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
> +	if (!drm_rect_intersect(&src_clip, &damage_clip))
> +		return;
> +
> +	dst_clip = plane_state->dst;
> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
>  		return;
>  
>  	if (!drm_dev_enter(dev, &idx))
>  		return;
>  
> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>  
>  	drm_dev_exit(idx);
>  }
> @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
>  	if (ret)
>  		return ret;
>  
> +	drm_plane_enable_fb_damage_clips(&pipe->plane);
> +
>  	drm_mode_config_reset(dev);
>  
>  	return 0;
> 
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 4732 bytes --]

Hi

Am 24.10.21 um 17:20 schrieb Noralf Trønnes:
> 
> 
> Den 22.10.2021 15.28, skrev Thomas Zimmermann:
>> Enable the FB_DAMAGE_CLIPS property to reduce display-update
>> overhead. Also fixes a warning in the kernel log.
>>
>>    simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called
>>
>> Fix the computation of the blit rectangle. This wasn't an issue so
>> far, as simpledrm always blitted the full framebuffer. The code now
>> supports damage clipping and virtual screen sizes.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++--------
>>   1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
>> index 571f716ff427..e872121e9fb0 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>>   	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
>>   	struct drm_device *dev = &sdev->dev;
>>   	void __iomem *dst = sdev->screen_base;
>> -	struct drm_rect clip;
>> +	struct drm_rect src_clip, dst_clip;
>>   	int idx;
>>   
>>   	if (!fb)
>> @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>>   	if (!drm_dev_enter(dev, &idx))
>>   		return;
>>   
>> -	drm_rect_init(&clip, 0, 0, fb->width, fb->height);
>> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
>>   
>> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
>> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
>> +	dst_clip = plane_state->dst;
>> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
>> +		return;
>> +
>> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
>> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>>   
>>   	drm_dev_exit(idx);
>>   }
>> @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
>>   	struct drm_framebuffer *fb = plane_state->fb;
>>   	struct drm_device *dev = &sdev->dev;
>>   	void __iomem *dst = sdev->screen_base;
>> -	struct drm_rect clip;
>> +	struct drm_rect damage_clip, src_clip, dst_clip;
>>   	int idx;
>>   
>>   	if (!fb)
>>   		return;
>>   
>> -	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip))
>> +	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip))
>> +		return;
>> +
> 
> I'm afraid I don't understand what's going on here, but isn't it
> possible to put this logic into drm_atomic_helper_damage_merged()?

The code above gets the damage rectangle (i.e., the plane's area that 
needs to be updated). The code below get's the framebuffer area. If they 
don't overlap, return. (I think this can really only fail with the next 
patch in the series, which adds virtual screens.)

> 
> Noralf.
> 
>> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
>> +	if (!drm_rect_intersect(&src_clip, &damage_clip))
>> +		return;
>> +
>> +	dst_clip = plane_state->dst;
>> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
>>   		return;

And here we check if the updated plane/framebuffer area is actually 
visible on screen; otherwise return. It could be in an area that is 
off-screen. (Again, this probably only fails with the virtual-screen patch.)

I don't think this is all generic enough to be within 
drm_atomic_helper_damage_merged(). But once we have multiple SHMEM-based 
drivers with virtual screens, we can move it into a helper for 
shadow-buffered planes. Your gud driver would be a candidate for this 
feature as well.

Best regards
Thomas

>>   
>>   	if (!drm_dev_enter(dev, &idx))
>>   		return;
>>   
>> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
>> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
>> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
>> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>>   
>>   	drm_dev_exit(idx);
>>   }
>> @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	drm_plane_enable_fb_damage_clips(&pipe->plane);
>> +
>>   	drm_mode_config_reset(dev);
>>   
>>   	return 0;
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property

[Thomas Zimmermann]

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

Hi

Am 24.10.21 um 17:20 schrieb Noralf Trønnes:
> 
> 
> Den 22.10.2021 15.28, skrev Thomas Zimmermann:
>> Enable the FB_DAMAGE_CLIPS property to reduce display-update
>> overhead. Also fixes a warning in the kernel log.
>>
>>    simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called
>>
>> Fix the computation of the blit rectangle. This wasn't an issue so
>> far, as simpledrm always blitted the full framebuffer. The code now
>> supports damage clipping and virtual screen sizes.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++--------
>>   1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
>> index 571f716ff427..e872121e9fb0 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>>   	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
>>   	struct drm_device *dev = &sdev->dev;
>>   	void __iomem *dst = sdev->screen_base;
>> -	struct drm_rect clip;
>> +	struct drm_rect src_clip, dst_clip;
>>   	int idx;
>>   
>>   	if (!fb)
>> @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>>   	if (!drm_dev_enter(dev, &idx))
>>   		return;
>>   
>> -	drm_rect_init(&clip, 0, 0, fb->width, fb->height);
>> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
>>   
>> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
>> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
>> +	dst_clip = plane_state->dst;
>> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
>> +		return;
>> +
>> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
>> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>>   
>>   	drm_dev_exit(idx);
>>   }
>> @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
>>   	struct drm_framebuffer *fb = plane_state->fb;
>>   	struct drm_device *dev = &sdev->dev;
>>   	void __iomem *dst = sdev->screen_base;
>> -	struct drm_rect clip;
>> +	struct drm_rect damage_clip, src_clip, dst_clip;
>>   	int idx;
>>   
>>   	if (!fb)
>>   		return;
>>   
>> -	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip))
>> +	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip))
>> +		return;
>> +
> 
> I'm afraid I don't understand what's going on here, but isn't it
> possible to put this logic into drm_atomic_helper_damage_merged()?

The code above gets the damage rectangle (i.e., the plane's area that 
needs to be updated). The code below get's the framebuffer area. If they 
don't overlap, return. (I think this can really only fail with the next 
patch in the series, which adds virtual screens.)

> 
> Noralf.
> 
>> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
>> +	if (!drm_rect_intersect(&src_clip, &damage_clip))
>> +		return;
>> +
>> +	dst_clip = plane_state->dst;
>> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
>>   		return;

And here we check if the updated plane/framebuffer area is actually 
visible on screen; otherwise return. It could be in an area that is 
off-screen. (Again, this probably only fails with the virtual-screen patch.)

I don't think this is all generic enough to be within 
drm_atomic_helper_damage_merged(). But once we have multiple SHMEM-based 
drivers with virtual screens, we can move it into a helper for 
shadow-buffered planes. Your gud driver would be a candidate for this 
feature as well.

Best regards
Thomas

>>   
>>   	if (!drm_dev_enter(dev, &idx))
>>   		return;
>>   
>> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
>> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
>> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
>> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>>   
>>   	drm_dev_exit(idx);
>>   }
>> @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	drm_plane_enable_fb_damage_clips(&pipe->plane);
>> +
>>   	drm_mode_config_reset(dev);
>>   
>>   	return 0;
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

Re: [PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property

[Thomas Zimmermann]

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

Hi

Am 24.10.21 um 17:20 schrieb Noralf Trønnes:
> 
> 
> Den 22.10.2021 15.28, skrev Thomas Zimmermann:
>> Enable the FB_DAMAGE_CLIPS property to reduce display-update
>> overhead. Also fixes a warning in the kernel log.
>>
>>    simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called
>>
>> Fix the computation of the blit rectangle. This wasn't an issue so
>> far, as simpledrm always blitted the full framebuffer. The code now
>> supports damage clipping and virtual screen sizes.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++--------
>>   1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
>> index 571f716ff427..e872121e9fb0 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>>   	void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */
>>   	struct drm_device *dev = &sdev->dev;
>>   	void __iomem *dst = sdev->screen_base;
>> -	struct drm_rect clip;
>> +	struct drm_rect src_clip, dst_clip;
>>   	int idx;
>>   
>>   	if (!fb)
>> @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
>>   	if (!drm_dev_enter(dev, &idx))
>>   		return;
>>   
>> -	drm_rect_init(&clip, 0, 0, fb->width, fb->height);
>> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
>>   
>> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
>> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
>> +	dst_clip = plane_state->dst;
>> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
>> +		return;
>> +
>> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
>> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>>   
>>   	drm_dev_exit(idx);
>>   }
>> @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
>>   	struct drm_framebuffer *fb = plane_state->fb;
>>   	struct drm_device *dev = &sdev->dev;
>>   	void __iomem *dst = sdev->screen_base;
>> -	struct drm_rect clip;
>> +	struct drm_rect damage_clip, src_clip, dst_clip;
>>   	int idx;
>>   
>>   	if (!fb)
>>   		return;
>>   
>> -	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip))
>> +	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip))
>> +		return;
>> +
> 
> I'm afraid I don't understand what's going on here, but isn't it
> possible to put this logic into drm_atomic_helper_damage_merged()?

The code above gets the damage rectangle (i.e., the plane's area that 
needs to be updated). The code below get's the framebuffer area. If they 
don't overlap, return. (I think this can really only fail with the next 
patch in the series, which adds virtual screens.)

> 
> Noralf.
> 
>> +	drm_rect_fp_to_int(&src_clip, &plane_state->src);
>> +	if (!drm_rect_intersect(&src_clip, &damage_clip))
>> +		return;
>> +
>> +	dst_clip = plane_state->dst;
>> +	if (!drm_rect_intersect(&dst_clip, &src_clip))
>>   		return;

And here we check if the updated plane/framebuffer area is actually 
visible on screen; otherwise return. It could be in an area that is 
off-screen. (Again, this probably only fails with the virtual-screen patch.)

I don't think this is all generic enough to be within 
drm_atomic_helper_damage_merged(). But once we have multiple SHMEM-based 
drivers with virtual screens, we can move it into a helper for 
shadow-buffered planes. Your gud driver would be a candidate for this 
feature as well.

Best regards
Thomas

>>   
>>   	if (!drm_dev_enter(dev, &idx))
>>   		return;
>>   
>> -	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip);
>> -	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip);
>> +	dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip);
>> +	drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip);
>>   
>>   	drm_dev_exit(idx);
>>   }
>> @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	drm_plane_enable_fb_damage_clips(&pipe->plane);
>> +
>>   	drm_mode_config_reset(dev);
>>   
>>   	return 0;
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

[PATCH 8/9] drm/simpledrm: Support virtual screen sizes

[Thomas Zimmermann]

Add constants for the maximum size of the shadow-plane surface
size. Useful for shadow planes with virtual screen sizes. The
current sizes are 4096 scanlines with 4096 pixels each. This
seems reasonable for current hardware, but can be increased as
necessary.

In simpledrm, set the maximum framebuffer size from the constants
for shadow planes. Implements support for virtual screen sizes and
page flipping on the fbdev console.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/simpledrm.c    |  9 +++++++--
 include/drm/drm_gem_atomic_helper.h | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index e872121e9fb0..e42ae1c6ebcd 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -2,6 +2,7 @@
 
 #include <linux/clk.h>
 #include <linux/of_clk.h>
+#include <linux/minmax.h>
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
@@ -776,6 +777,7 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
 	struct drm_display_mode *mode = &sdev->mode;
 	struct drm_connector *connector = &sdev->connector;
 	struct drm_simple_display_pipe *pipe = &sdev->pipe;
+	unsigned long max_width, max_height;
 	const uint32_t *formats;
 	size_t nformats;
 	int ret;
@@ -784,10 +786,13 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
 	if (ret)
 		return ret;
 
+	max_width = max_t(unsigned long, mode->hdisplay, DRM_SHADOW_PLANE_MAX_WIDTH);
+	max_height = max_t(unsigned long, mode->vdisplay, DRM_SHADOW_PLANE_MAX_HEIGHT);
+
 	dev->mode_config.min_width = mode->hdisplay;
-	dev->mode_config.max_width = mode->hdisplay;
+	dev->mode_config.max_width = max_width;
 	dev->mode_config.min_height = mode->vdisplay;
-	dev->mode_config.max_height = mode->vdisplay;
+	dev->mode_config.max_height = max_height;
 	dev->mode_config.prefer_shadow_fbdev = true;
 	dev->mode_config.preferred_depth = sdev->format->cpp[0] * 8;
 	dev->mode_config.funcs = &simpledrm_mode_config_funcs;
diff --git a/include/drm/drm_gem_atomic_helper.h b/include/drm/drm_gem_atomic_helper.h
index 48222a107873..54983ecf641a 100644
--- a/include/drm/drm_gem_atomic_helper.h
+++ b/include/drm/drm_gem_atomic_helper.h
@@ -22,6 +22,24 @@ int drm_gem_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
  * Helpers for planes with shadow buffers
  */
 
+/**
+ * DRM_SHADOW_PLANE_MAX_WIDTH - Maximum width of a plane's shadow buffer in pixels
+ *
+ * For drivers with shadow planes, the maximum width of the framebuffer is
+ * usually independent from hardware limitations. Drivers can initialize struct
+ * drm_mode_config.max_width from DRM_SHADOW_PLANE_MAX_WIDTH.
+ */
+#define DRM_SHADOW_PLANE_MAX_WIDTH	(1ul << 12)
+
+/**
+ * DRM_SHADOW_PLANE_MAX_HEIGHT - Maximum height of a plane's shadow buffer in scanlines
+ *
+ * For drivers with shadow planes, the maximum height of the framebuffer is
+ * usually independent from hardware limitations. Drivers can initialize struct
+ * drm_mode_config.max_height from DRM_SHADOW_PLANE_MAX_HEIGHT.
+ */
+#define DRM_SHADOW_PLANE_MAX_HEIGHT	(1ul << 12)
+
 /**
  * struct drm_shadow_plane_state - plane state for planes with shadow buffers
  *
-- 
2.33.0

[PATCH 8/9] drm/simpledrm: Support virtual screen sizes

[Thomas Zimmermann]

Add constants for the maximum size of the shadow-plane surface
size. Useful for shadow planes with virtual screen sizes. The
current sizes are 4096 scanlines with 4096 pixels each. This
seems reasonable for current hardware, but can be increased as
necessary.

In simpledrm, set the maximum framebuffer size from the constants
for shadow planes. Implements support for virtual screen sizes and
page flipping on the fbdev console.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/simpledrm.c    |  9 +++++++--
 include/drm/drm_gem_atomic_helper.h | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index e872121e9fb0..e42ae1c6ebcd 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -2,6 +2,7 @@
 
 #include <linux/clk.h>
 #include <linux/of_clk.h>
+#include <linux/minmax.h>
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
@@ -776,6 +777,7 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
 	struct drm_display_mode *mode = &sdev->mode;
 	struct drm_connector *connector = &sdev->connector;
 	struct drm_simple_display_pipe *pipe = &sdev->pipe;
+	unsigned long max_width, max_height;
 	const uint32_t *formats;
 	size_t nformats;
 	int ret;
@@ -784,10 +786,13 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev)
 	if (ret)
 		return ret;
 
+	max_width = max_t(unsigned long, mode->hdisplay, DRM_SHADOW_PLANE_MAX_WIDTH);
+	max_height = max_t(unsigned long, mode->vdisplay, DRM_SHADOW_PLANE_MAX_HEIGHT);
+
 	dev->mode_config.min_width = mode->hdisplay;
-	dev->mode_config.max_width = mode->hdisplay;
+	dev->mode_config.max_width = max_width;
 	dev->mode_config.min_height = mode->vdisplay;
-	dev->mode_config.max_height = mode->vdisplay;
+	dev->mode_config.max_height = max_height;
 	dev->mode_config.prefer_shadow_fbdev = true;
 	dev->mode_config.preferred_depth = sdev->format->cpp[0] * 8;
 	dev->mode_config.funcs = &simpledrm_mode_config_funcs;
diff --git a/include/drm/drm_gem_atomic_helper.h b/include/drm/drm_gem_atomic_helper.h
index 48222a107873..54983ecf641a 100644
--- a/include/drm/drm_gem_atomic_helper.h
+++ b/include/drm/drm_gem_atomic_helper.h
@@ -22,6 +22,24 @@ int drm_gem_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
  * Helpers for planes with shadow buffers
  */
 
+/**
+ * DRM_SHADOW_PLANE_MAX_WIDTH - Maximum width of a plane's shadow buffer in pixels
+ *
+ * For drivers with shadow planes, the maximum width of the framebuffer is
+ * usually independent from hardware limitations. Drivers can initialize struct
+ * drm_mode_config.max_width from DRM_SHADOW_PLANE_MAX_WIDTH.
+ */
+#define DRM_SHADOW_PLANE_MAX_WIDTH	(1ul << 12)
+
+/**
+ * DRM_SHADOW_PLANE_MAX_HEIGHT - Maximum height of a plane's shadow buffer in scanlines
+ *
+ * For drivers with shadow planes, the maximum height of the framebuffer is
+ * usually independent from hardware limitations. Drivers can initialize struct
+ * drm_mode_config.max_height from DRM_SHADOW_PLANE_MAX_HEIGHT.
+ */
+#define DRM_SHADOW_PLANE_MAX_HEIGHT	(1ul << 12)
+
 /**
  * struct drm_shadow_plane_state - plane state for planes with shadow buffers
  *
-- 
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 9/9] drm: Clarify semantics of struct drm_mode_config.{min,max}_{width,height}

[Thomas Zimmermann]

Add additional information on the semantics of the size fields in
struct drm_mode_config. Also add a TODO to review all driver for
correct usage of these fields.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 Documentation/gpu/todo.rst    | 15 +++++++++++++++
 include/drm/drm_mode_config.h | 13 +++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 60d1d7ee0719..f4e1d72149f7 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -463,6 +463,21 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>, Christian König, Daniel Vette
 
 Level: Intermediate
 
+Review all drivers for setting struct drm_mode_config.{max_width,max_height} correctly
+--------------------------------------------------------------------------------------
+
+The values in struct drm_mode_config.{max_width,max_height} describe the
+maximum supported framebuffer size. It's the virtual screen size, but many
+drivers treat it like limitations of the physical resolution.
+
+The maximum width depends on the hardware's maximum scanline pitch. The
+maximum height depends on the amount of addressable video memory. Review all
+drivers to initialize the fields to the correct values.
+
+Contact: Thomas Zimmermann <tzimmermann@suse.de>
+
+Level: Intermediate
+
 
 Core refactorings
 =================
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 48b7de80daf5..91ca575a78de 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -359,6 +359,19 @@ struct drm_mode_config_funcs {
  * Core mode resource tracking structure.  All CRTC, encoders, and connectors
  * enumerated by the driver are added here, as are global properties.  Some
  * global restrictions are also here, e.g. dimension restrictions.
+ *
+ * Framebuffer sizes refer to the virtual screen that can be displayed by
+ * the CRTC. This can be different from the physical resolution programmed.
+ * The minimum width and height, stored in @min_width and @min_height,
+ * describe the smallest size of the framebuffer. It correlates to the
+ * minimum programmable resolution.
+ * The maximum width, stored in @max_width, is typically limited by the
+ * maximum pitch between two adjacent scanlines. The maximum height, stored
+ * in @max_height, is usually only limited by the amount of addressable video
+ * memory. For hardware that has no real maximum, drivers should pick a
+ * reasonable default.
+ *
+ * See also @DRM_SHADOW_PLANE_MAX_WIDTH and @DRM_SHADOW_PLANE_MAX_HEIGHT.
  */
 struct drm_mode_config {
 	/**
-- 
2.33.0

[PATCH 9/9] drm: Clarify semantics of struct drm_mode_config.{min, max}_{width, height}

[Thomas Zimmermann]

Add additional information on the semantics of the size fields in
struct drm_mode_config. Also add a TODO to review all driver for
correct usage of these fields.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 Documentation/gpu/todo.rst    | 15 +++++++++++++++
 include/drm/drm_mode_config.h | 13 +++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 60d1d7ee0719..f4e1d72149f7 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -463,6 +463,21 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>, Christian König, Daniel Vette
 
 Level: Intermediate
 
+Review all drivers for setting struct drm_mode_config.{max_width,max_height} correctly
+--------------------------------------------------------------------------------------
+
+The values in struct drm_mode_config.{max_width,max_height} describe the
+maximum supported framebuffer size. It's the virtual screen size, but many
+drivers treat it like limitations of the physical resolution.
+
+The maximum width depends on the hardware's maximum scanline pitch. The
+maximum height depends on the amount of addressable video memory. Review all
+drivers to initialize the fields to the correct values.
+
+Contact: Thomas Zimmermann <tzimmermann@suse.de>
+
+Level: Intermediate
+
 
 Core refactorings
 =================
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 48b7de80daf5..91ca575a78de 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -359,6 +359,19 @@ struct drm_mode_config_funcs {
  * Core mode resource tracking structure.  All CRTC, encoders, and connectors
  * enumerated by the driver are added here, as are global properties.  Some
  * global restrictions are also here, e.g. dimension restrictions.
+ *
+ * Framebuffer sizes refer to the virtual screen that can be displayed by
+ * the CRTC. This can be different from the physical resolution programmed.
+ * The minimum width and height, stored in @min_width and @min_height,
+ * describe the smallest size of the framebuffer. It correlates to the
+ * minimum programmable resolution.
+ * The maximum width, stored in @max_width, is typically limited by the
+ * maximum pitch between two adjacent scanlines. The maximum height, stored
+ * in @max_height, is usually only limited by the amount of addressable video
+ * memory. For hardware that has no real maximum, drivers should pick a
+ * reasonable default.
+ *
+ * See also @DRM_SHADOW_PLANE_MAX_WIDTH and @DRM_SHADOW_PLANE_MAX_HEIGHT.
  */
 struct drm_mode_config {
 	/**
-- 
2.33.0

[PATCH 9/9] drm: Clarify semantics of struct drm_mode_config.{min, max}_{width, height}

[Thomas Zimmermann]

Add additional information on the semantics of the size fields in
struct drm_mode_config. Also add a TODO to review all driver for
correct usage of these fields.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 Documentation/gpu/todo.rst    | 15 +++++++++++++++
 include/drm/drm_mode_config.h | 13 +++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 60d1d7ee0719..f4e1d72149f7 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -463,6 +463,21 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>, Christian König, Daniel Vette
 
 Level: Intermediate
 
+Review all drivers for setting struct drm_mode_config.{max_width,max_height} correctly
+--------------------------------------------------------------------------------------
+
+The values in struct drm_mode_config.{max_width,max_height} describe the
+maximum supported framebuffer size. It's the virtual screen size, but many
+drivers treat it like limitations of the physical resolution.
+
+The maximum width depends on the hardware's maximum scanline pitch. The
+maximum height depends on the amount of addressable video memory. Review all
+drivers to initialize the fields to the correct values.
+
+Contact: Thomas Zimmermann <tzimmermann@suse.de>
+
+Level: Intermediate
+
 
 Core refactorings
 =================
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 48b7de80daf5..91ca575a78de 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -359,6 +359,19 @@ struct drm_mode_config_funcs {
  * Core mode resource tracking structure.  All CRTC, encoders, and connectors
  * enumerated by the driver are added here, as are global properties.  Some
  * global restrictions are also here, e.g. dimension restrictions.
+ *
+ * Framebuffer sizes refer to the virtual screen that can be displayed by
+ * the CRTC. This can be different from the physical resolution programmed.
+ * The minimum width and height, stored in @min_width and @min_height,
+ * describe the smallest size of the framebuffer. It correlates to the
+ * minimum programmable resolution.
+ * The maximum width, stored in @max_width, is typically limited by the
+ * maximum pitch between two adjacent scanlines. The maximum height, stored
+ * in @max_height, is usually only limited by the amount of addressable video
+ * memory. For hardware that has no real maximum, drivers should pick a
+ * reasonable default.
+ *
+ * See also @DRM_SHADOW_PLANE_MAX_WIDTH and @DRM_SHADOW_PLANE_MAX_HEIGHT.
  */
 struct drm_mode_config {
 	/**
-- 
2.33.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 0/9] drm/simpledrm: Enable damage clips and virtual screens

[Sam Ravnborg]

Hi Thomas,

On Fri, Oct 22, 2021 at 03:28:20PM +0200, Thomas Zimmermann wrote:
> Enable FB_DAMAGE_CLIPS with simpledrm for improved performance and/or
> less overhead. With this in place, add support for virtual screens
> (i.e., framebuffers that are larger than the display resolution). This
> also enables fbdev panning and page flipping.
> 
> After the discussion and bug fixing wrt to fbdev overallocation, I
> decided to add full support for this to simpledrm. Patches #1 to #5
> change the format-helper functions accordingly. Destination buffers
> are now clipped by the caller and all functions support a similar
> feature set. This has some fallout in various drivers.
> 
> Patch #6 change fbdev emulation to support overallocation with
> shadow buffers, even if the hardware buffer would be too small.
This change is very welcome - I hope it will solve the problem
Alistair experience - see:
https://lore.kernel.org/all/CAKmqyKPCP45O5_gjCFwUs8jU4NrDnjAeLs7OYAE4j-LEUw+Hzg@mail.gmail.com/

	Sam

> 
> Patch #7 and #8 update simpledrm to enable damage clipping and virtual
> screen sizes. Both feature go hand in hand, sort of. For shadow-
> buffered planes, the DRM framebuffer lives in system memory. So the
> maximum size of the virtual screen is somewhat arbitrary. We add two
> constants for resonable maximum width and height of 4096 each.
> 
> Patch #9 adds documentation and a TODO item.
> 
> Tested with simpledrm. I also ran the recently posted fbdev panning
> tests to make sure that the fbdev overallocation works correctly. [1]
> 
> [1] https://lists.freedesktop.org/archives/igt-dev/2021-October/036642.html
> 
> Thomas Zimmermann (9):
>   drm/format-helper: Export drm_fb_clip_offset()
>   drm/format-helper: Rework format-helper memcpy functions
>   drm/format-helper: Add destination-buffer pitch to drm_fb_swab()
>   drm/format-helper: Rework format-helper conversion functions
>   drm/format-helper: Streamline blit-helper interface
>   drm/fb-helper: Allocate shadow buffer of surface height
>   drm/simpledrm: Enable FB_DAMAGE_CLIPS property
>   drm/simpledrm: Support virtual screen sizes
>   drm: Clarify semantics of struct
>     drm_mode_config.{min,max}_{width,height}
> 
>  Documentation/gpu/todo.rst                  |  15 ++
>  drivers/gpu/drm/drm_fb_helper.c             |   2 +-
>  drivers/gpu/drm/drm_format_helper.c         | 236 ++++++++++----------
>  drivers/gpu/drm/drm_mipi_dbi.c              |   6 +-
>  drivers/gpu/drm/gud/gud_pipe.c              |  14 +-
>  drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |   5 +-
>  drivers/gpu/drm/mgag200/mgag200_mode.c      |   4 +-
>  drivers/gpu/drm/tiny/cirrus.c               |  24 +-
>  drivers/gpu/drm/tiny/repaper.c              |   2 +-
>  drivers/gpu/drm/tiny/simpledrm.c            |  41 +++-
>  drivers/gpu/drm/tiny/st7586.c               |   2 +-
>  include/drm/drm_format_helper.h             |  58 ++---
>  include/drm/drm_gem_atomic_helper.h         |  18 ++
>  include/drm/drm_mode_config.h               |  13 ++
>  14 files changed, 254 insertions(+), 186 deletions(-)
> 
> --
> 2.33.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 0/9] drm/simpledrm: Enable damage clips and virtual screens

[Sam Ravnborg]

Hi Thomas,

On Fri, Oct 22, 2021 at 03:28:20PM +0200, Thomas Zimmermann wrote:
> Enable FB_DAMAGE_CLIPS with simpledrm for improved performance and/or
> less overhead. With this in place, add support for virtual screens
> (i.e., framebuffers that are larger than the display resolution). This
> also enables fbdev panning and page flipping.
> 
> After the discussion and bug fixing wrt to fbdev overallocation, I
> decided to add full support for this to simpledrm. Patches #1 to #5
> change the format-helper functions accordingly. Destination buffers
> are now clipped by the caller and all functions support a similar
> feature set. This has some fallout in various drivers.
> 
> Patch #6 change fbdev emulation to support overallocation with
> shadow buffers, even if the hardware buffer would be too small.
This change is very welcome - I hope it will solve the problem
Alistair experience - see:
https://lore.kernel.org/all/CAKmqyKPCP45O5_gjCFwUs8jU4NrDnjAeLs7OYAE4j-LEUw+Hzg@mail.gmail.com/

	Sam

> 
> Patch #7 and #8 update simpledrm to enable damage clipping and virtual
> screen sizes. Both feature go hand in hand, sort of. For shadow-
> buffered planes, the DRM framebuffer lives in system memory. So the
> maximum size of the virtual screen is somewhat arbitrary. We add two
> constants for resonable maximum width and height of 4096 each.
> 
> Patch #9 adds documentation and a TODO item.
> 
> Tested with simpledrm. I also ran the recently posted fbdev panning
> tests to make sure that the fbdev overallocation works correctly. [1]
> 
> [1] https://lists.freedesktop.org/archives/igt-dev/2021-October/036642.html
> 
> Thomas Zimmermann (9):
>   drm/format-helper: Export drm_fb_clip_offset()
>   drm/format-helper: Rework format-helper memcpy functions
>   drm/format-helper: Add destination-buffer pitch to drm_fb_swab()
>   drm/format-helper: Rework format-helper conversion functions
>   drm/format-helper: Streamline blit-helper interface
>   drm/fb-helper: Allocate shadow buffer of surface height
>   drm/simpledrm: Enable FB_DAMAGE_CLIPS property
>   drm/simpledrm: Support virtual screen sizes
>   drm: Clarify semantics of struct
>     drm_mode_config.{min,max}_{width,height}
> 
>  Documentation/gpu/todo.rst                  |  15 ++
>  drivers/gpu/drm/drm_fb_helper.c             |   2 +-
>  drivers/gpu/drm/drm_format_helper.c         | 236 ++++++++++----------
>  drivers/gpu/drm/drm_mipi_dbi.c              |   6 +-
>  drivers/gpu/drm/gud/gud_pipe.c              |  14 +-
>  drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |   5 +-
>  drivers/gpu/drm/mgag200/mgag200_mode.c      |   4 +-
>  drivers/gpu/drm/tiny/cirrus.c               |  24 +-
>  drivers/gpu/drm/tiny/repaper.c              |   2 +-
>  drivers/gpu/drm/tiny/simpledrm.c            |  41 +++-
>  drivers/gpu/drm/tiny/st7586.c               |   2 +-
>  include/drm/drm_format_helper.h             |  58 ++---
>  include/drm/drm_gem_atomic_helper.h         |  18 ++
>  include/drm/drm_mode_config.h               |  13 ++
>  14 files changed, 254 insertions(+), 186 deletions(-)
> 
> --
> 2.33.0