Re: [PATCH v5 1/8] drm/omap: Add ability to check if requested plane modes can be supported

[Neil Armstrong <narmstrong@baylibre.com>]

On 12/10/2021 09:21, Tomi Valkeinen wrote:
> Hi,
> 
> On 23/09/2021 10:06, Neil Armstrong wrote:
>> From: Benoit Parrot <bparrot@ti.com>
>>
>> We currently assume that an overlay has the same maximum width and
>> maximum height as the overlay manager. This assumption is incorrect. On
>> some variants the overlay manager maximum width is twice the maximum
>> width that the overlay can handle. We need to add the appropriate data
>> per variant as well as export a helper function to retrieve the data so
>> check can be made dynamically in omap_plane_atomic_check().
>>
>> Signed-off-by: Benoit Parrot <bparrot@ti.com>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>   drivers/gpu/drm/omapdrm/dss/dispc.c  | 22 ++++++++++++++++++++++
>>   drivers/gpu/drm/omapdrm/dss/dss.h    |  2 ++
>>   drivers/gpu/drm/omapdrm/omap_plane.c | 14 ++++++++++++++
>>   3 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
>> index 3c4a4991e45a..bdecec8f4d88 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
>> @@ -92,6 +92,8 @@ struct dispc_features {
>>       u8 mgr_height_start;
>>       u16 mgr_width_max;
>>       u16 mgr_height_max;
>> +    u16 ovl_width_max;
>> +    u16 ovl_height_max;
>>       unsigned long max_lcd_pclk;
>>       unsigned long max_tv_pclk;
>>       unsigned int max_downscale;
>> @@ -2599,6 +2601,12 @@ static int dispc_ovl_calc_scaling(struct dispc_device *dispc,
>>       return 0;
>>   }
>>   +void dispc_ovl_get_max_size(struct dispc_device *dispc, u16 *width, u16 *height)
>> +{
>> +    *width = dispc->feat->ovl_width_max;
>> +    *height = dispc->feat->ovl_height_max;
>> +}
>> +
>>   static int dispc_ovl_setup_common(struct dispc_device *dispc,
>>                     enum omap_plane_id plane,
>>                     enum omap_overlay_caps caps,
>> @@ -4240,6 +4248,8 @@ static const struct dispc_features omap24xx_dispc_feats = {
>>       .mgr_height_start    =    26,
>>       .mgr_width_max        =    2048,
>>       .mgr_height_max        =    2048,
>> +    .ovl_width_max        =    2048,
>> +    .ovl_height_max        =    2048,
>>       .max_lcd_pclk        =    66500000,
>>       .max_downscale        =    2,
>>       /*
>> @@ -4278,6 +4288,8 @@ static const struct dispc_features omap34xx_rev1_0_dispc_feats = {
>>       .mgr_height_start    =    26,
>>       .mgr_width_max        =    2048,
>>       .mgr_height_max        =    2048,
>> +    .ovl_width_max        =    2048,
>> +    .ovl_height_max        =    2048,
>>       .max_lcd_pclk        =    173000000,
>>       .max_tv_pclk        =    59000000,
>>       .max_downscale        =    4,
>> @@ -4313,6 +4325,8 @@ static const struct dispc_features omap34xx_rev3_0_dispc_feats = {
>>       .mgr_height_start    =    26,
>>       .mgr_width_max        =    2048,
>>       .mgr_height_max        =    2048,
>> +    .ovl_width_max        =    2048,
>> +    .ovl_height_max        =    2048,
>>       .max_lcd_pclk        =    173000000,
>>       .max_tv_pclk        =    59000000,
>>       .max_downscale        =    4,
>> @@ -4348,6 +4362,8 @@ static const struct dispc_features omap36xx_dispc_feats = {
>>       .mgr_height_start    =    26,
>>       .mgr_width_max        =    2048,
>>       .mgr_height_max        =    2048,
>> +    .ovl_width_max        =    2048,
>> +    .ovl_height_max        =    2048,
>>       .max_lcd_pclk        =    173000000,
>>       .max_tv_pclk        =    59000000,
>>       .max_downscale        =    4,
>> @@ -4383,6 +4399,8 @@ static const struct dispc_features am43xx_dispc_feats = {
>>       .mgr_height_start    =    26,
>>       .mgr_width_max        =    2048,
>>       .mgr_height_max        =    2048,
>> +    .ovl_width_max        =    2048,
>> +    .ovl_height_max        =    2048,
>>       .max_lcd_pclk        =    173000000,
>>       .max_tv_pclk        =    59000000,
>>       .max_downscale        =    4,
>> @@ -4418,6 +4436,8 @@ static const struct dispc_features omap44xx_dispc_feats = {
>>       .mgr_height_start    =    26,
>>       .mgr_width_max        =    2048,
>>       .mgr_height_max        =    2048,
>> +    .ovl_width_max        =    2048,
>> +    .ovl_height_max        =    2048,
>>       .max_lcd_pclk        =    170000000,
>>       .max_tv_pclk        =    185625000,
>>       .max_downscale        =    4,
>> @@ -4457,6 +4477,8 @@ static const struct dispc_features omap54xx_dispc_feats = {
>>       .mgr_height_start    =    27,
>>       .mgr_width_max        =    4096,
>>       .mgr_height_max        =    4096,
>> +    .ovl_width_max        =    2048,
>> +    .ovl_height_max        =    4096,
>>       .max_lcd_pclk        =    170000000,
>>       .max_tv_pclk        =    192000000,
>>       .max_downscale        =    4,
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h
>> index a547527bb2f3..14c39f7c3988 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dss.h
>> +++ b/drivers/gpu/drm/omapdrm/dss/dss.h
>> @@ -397,6 +397,8 @@ int dispc_get_num_mgrs(struct dispc_device *dispc);
>>   const u32 *dispc_ovl_get_color_modes(struct dispc_device *dispc,
>>                           enum omap_plane_id plane);
>>   +void dispc_ovl_get_max_size(struct dispc_device *dispc, u16 *width, u16 *height);
>> +
>>   u32 dispc_read_irqstatus(struct dispc_device *dispc);
>>   void dispc_clear_irqstatus(struct dispc_device *dispc, u32 mask);
>>   void dispc_write_irqenable(struct dispc_device *dispc, u32 mask);
>> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
>> index 512af976b7e9..d0a67b7ed1a0 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
>> @@ -109,11 +109,18 @@ static int omap_plane_atomic_check(struct drm_plane *plane,
>>   {
>>       struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
>>                                            plane);
>> +    struct omap_drm_private *priv = plane->dev->dev_private;
>>       struct drm_crtc_state *crtc_state;
>> +    u16 width, height;
>> +    u32 width_fp, height_fp;
> 
> I think naming these max_w/max_width etc. would be better.

Ack

Neil

> 
>  Tomi