Re: [PATCH 4/6] drm/omap: Allow allocation of larger buffers

[Archit Taneja <archit@ti.com>]

On Monday 14 April 2014 02:55 PM, Tomi Valkeinen wrote:
> On 11/04/14 10:23, Archit Taneja wrote:
>> The drm ioctl DRM_IOCTL_MODE_ADDFB2 doesn't let us allocate buffers which are
>> greater than what is specified in the driver through dev->mode_config.
>>
>> Create helpers for DISPC which return the max manager width and height supported
>> by the device. The maximum width for a framebuffer is set to the combined width
>> of the all the crtcs, assuming they are arranged horizontally.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>>   drivers/gpu/drm/omapdrm/omap_drv.c | 10 ++++++----
>>   drivers/video/omap2/dss/dispc.c    | 12 ++++++++++++
>>   include/video/omapdss.h            |  2 ++
>>   3 files changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
>> index c8270e4..55ec575 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> @@ -306,11 +306,13 @@ static int omap_modeset_init(struct drm_device *dev)
>>   	dev->mode_config.min_width = 32;
>>   	dev->mode_config.min_height = 32;
>>
>> -	/* note: eventually will need some cpu_is_omapXYZ() type stuff here
>> -	 * to fill in these limits properly on different OMAP generations..
>> +	/*
>> +	 * Note: the maximum width is set to the combined width of all the
>> +	 * crtcs. We could assume the same for the maximum height too, but
>> +	 * we generally don't use such a configuration.
>>   	 */
>> -	dev->mode_config.max_width = 2048;
>> -	dev->mode_config.max_height = 2048;
>> +	dev->mode_config.max_width = num_crtcs * dispc_mgr_max_width();
>> +	dev->mode_config.max_height = dispc_mgr_max_height();
>
> This looks very strange.
>
> If the max size is supposed to be the maximum output size we support,
> then multiplying with num_crtcs doesn't make sense.
>
> If, on the other hand, it tells the possible maximum size of the
> framebuffer in the memory, of which only small part is shown (where's
> the max size of that "part" defined, then?), then there should be no
> limits as the only limit is the size of the memory.

These parameters are used to tell the max size of framebuffer we can 
allocate in memory.

I'm not sure why there is a limit in the first place, but if we have a 
really low minimum(like 2048 pixels right now), we can't have multiple 
displays showing different regions of the same buffer.

Archit