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

[Noralf Trønnes]

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 at 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 at 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
>>>    *
>>>
>