diff mbox series

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

Message ID 20211101141532.26655-9-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/simpledrm: Enable damage clips and virtual screens | expand

Commit Message

Thomas Zimmermann Nov. 1, 2021, 2:15 p.m. UTC
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(-)

Comments

Noralf Trønnes Nov. 8, 2021, 9:01 p.m. UTC | #1
Den 01.11.2021 15.15, skrev 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.

Why would a driver do that instead of using a value of its own? Is it
some kind of standardization?

> + */
> +#define DRM_SHADOW_PLANE_MAX_WIDTH	(1ul << 12)

Please use a decimal number, I'm so slow at doing this in my head that I
use bash to calculate it for me, which really slows down parsing the code.

Noralf.

> +
> +/**
> + * 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
>   *
>
Thomas Zimmermann Nov. 9, 2021, 9:06 a.m. UTC | #2
Hi

Am 08.11.21 um 22:01 schrieb Noralf Trønnes:
> 
> 
> Den 01.11.2021 15.15, skrev 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.
> 
> Why would a driver do that instead of using a value of its own? Is it
> some kind of standardization?

Exactly. The shadow framebuffer is in system memory, so its size is 
arbitrarily large. If each driver sets its own limit, it just fragments 
the DRM feature set. There's usually no reason why one driver can have 
4096 pixels and another one just 2048 or even 8192. Setting a constant 
harmonizes this among drivers.

Please note that nothing really depends on this value. Drivers can still 
use a different limit if they have to.

> 
>> + */
>> +#define DRM_SHADOW_PLANE_MAX_WIDTH	(1ul << 12)
> 
> Please use a decimal number, I'm so slow at doing this in my head that I
> use bash to calculate it for me, which really slows down parsing the code.

Ok. :D

Best regard
Thomas

> 
> Noralf.
> 
>> +
>> +/**
>> + * 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
>>    *
>>
Noralf Trønnes Nov. 9, 2021, 1:05 p.m. UTC | #3
Den 09.11.2021 10.06, skrev Thomas Zimmermann:
> Hi
> 
> Am 08.11.21 um 22:01 schrieb Noralf Trønnes:
>>
>>
>> Den 01.11.2021 15.15, skrev 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.
>>
>> Why would a driver do that instead of using a value of its own? Is it
>> some kind of standardization?
> 
> Exactly. The shadow framebuffer is in system memory, so its size is
> arbitrarily large. If each driver sets its own limit, it just fragments
> the DRM feature set. There's usually no reason why one driver can have
> 4096 pixels and another one just 2048 or even 8192. Setting a constant
> harmonizes this among drivers.
> 
> Please note that nothing really depends on this value. Drivers can still
> use a different limit if they have to.
> 
>>
>>> + */
>>> +#define DRM_SHADOW_PLANE_MAX_WIDTH    (1ul << 12)
>>
>> Please use a decimal number, I'm so slow at doing this in my head that I
>> use bash to calculate it for me, which really slows down parsing the
>> code.
> 
> Ok. :D
> 

With that fixed:

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


> Best regard
> Thomas
> 
>>
>> Noralf.
>>
>>> +
>>> +/**
>>> + * 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
>>>    *
>>>
>
diff mbox series

Patch

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
  *