[PATCH] drm/meson: fix max mode_config height/width

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

The mode_config max_width/max_height determines the maximum framebuffer
size the pixel reader can handle. But the values were set thinking they
were determining the maximum screen dimensions.

This patch changes the values to the maximum height/width the CANVAS block
can handle rounded to some coherent values.

Fixes: a41e82e6c457 ("drm/meson: Add support for components")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/meson/meson_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index d344312..2e29968 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 		goto free_drm;
 
 	drm_mode_config_init(drm);
-	drm->mode_config.max_width = 3840;
-	drm->mode_config.max_height = 2160;
+	drm->mode_config.max_width = 16384;
+	drm->mode_config.max_height = 8192;
 	drm->mode_config.funcs = &meson_mode_config_funcs;
 
 	/* Hardware Initialization */
-- 
2.7.4

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

The mode_config max_width/max_height determines the maximum framebuffer
size the pixel reader can handle. But the values were set thinking they
were determining the maximum screen dimensions.

This patch changes the values to the maximum height/width the CANVAS block
can handle rounded to some coherent values.

Fixes: a41e82e6c457 ("drm/meson: Add support for components")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/meson/meson_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index d344312..2e29968 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 		goto free_drm;
 
 	drm_mode_config_init(drm);
-	drm->mode_config.max_width = 3840;
-	drm->mode_config.max_height = 2160;
+	drm->mode_config.max_width = 16384;
+	drm->mode_config.max_height = 8192;
 	drm->mode_config.funcs = &meson_mode_config_funcs;
 
 	/* Hardware Initialization */
-- 
2.7.4

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

The mode_config max_width/max_height determines the maximum framebuffer
size the pixel reader can handle. But the values were set thinking they
were determining the maximum screen dimensions.

This patch changes the values to the maximum height/width the CANVAS block
can handle rounded to some coherent values.

Fixes: a41e82e6c457 ("drm/meson: Add support for components")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/meson/meson_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index d344312..2e29968 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 		goto free_drm;
 
 	drm_mode_config_init(drm);
-	drm->mode_config.max_width = 3840;
-	drm->mode_config.max_height = 2160;
+	drm->mode_config.max_width = 16384;
+	drm->mode_config.max_height = 8192;
 	drm->mode_config.funcs = &meson_mode_config_funcs;
 
 	/* Hardware Initialization */
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

The mode_config max_width/max_height determines the maximum framebuffer
size the pixel reader can handle. But the values were set thinking they
were determining the maximum screen dimensions.

This patch changes the values to the maximum height/width the CANVAS block
can handle rounded to some coherent values.

Fixes: a41e82e6c457 ("drm/meson: Add support for components")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/meson/meson_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index d344312..2e29968 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 		goto free_drm;
 
 	drm_mode_config_init(drm);
-	drm->mode_config.max_width = 3840;
-	drm->mode_config.max_height = 2160;
+	drm->mode_config.max_width = 16384;
+	drm->mode_config.max_height = 8192;
 	drm->mode_config.funcs = &meson_mode_config_funcs;
 
 	/* Hardware Initialization */
-- 
2.7.4

[PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
> The mode_config max_width/max_height determines the maximum framebuffer
> size the pixel reader can handle. But the values were set thinking they
> were determining the maximum screen dimensions.
> 
> This patch changes the values to the maximum height/width the CANVAS block
> can handle rounded to some coherent values.
> 
> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

It's both. Grep for all the callers of ->fill_modes and you'll see that
this limit is also used to filter max screen sizes.

If you want to change this, then I think we need a new
mode_config.fb_max_width/height, which if non-zero, would extend the limit
for fbs.

There's also the problem that if you extend this for fbs, then there's no
check anymore in the atomic_commit paths (or legacy modeset), so that
needs to be addressed somehow too.

Bunch of igt to make sure we're not missing anything would be sweet on
top, e.g. e.g. trying to set a mode over the limit and making sure it
fails.

Cheers, Daniel

> ---
>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index d344312..2e29968 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  		goto free_drm;
>  
>  	drm_mode_config_init(drm);
> -	drm->mode_config.max_width = 3840;
> -	drm->mode_config.max_height = 2160;
> +	drm->mode_config.max_width = 16384;
> +	drm->mode_config.max_height = 8192;
>  	drm->mode_config.funcs = &meson_mode_config_funcs;
>  
>  	/* Hardware Initialization */
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
> The mode_config max_width/max_height determines the maximum framebuffer
> size the pixel reader can handle. But the values were set thinking they
> were determining the maximum screen dimensions.
> 
> This patch changes the values to the maximum height/width the CANVAS block
> can handle rounded to some coherent values.
> 
> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

It's both. Grep for all the callers of ->fill_modes and you'll see that
this limit is also used to filter max screen sizes.

If you want to change this, then I think we need a new
mode_config.fb_max_width/height, which if non-zero, would extend the limit
for fbs.

There's also the problem that if you extend this for fbs, then there's no
check anymore in the atomic_commit paths (or legacy modeset), so that
needs to be addressed somehow too.

Bunch of igt to make sure we're not missing anything would be sweet on
top, e.g. e.g. trying to set a mode over the limit and making sure it
fails.

Cheers, Daniel

> ---
>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index d344312..2e29968 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  		goto free_drm;
>  
>  	drm_mode_config_init(drm);
> -	drm->mode_config.max_width = 3840;
> -	drm->mode_config.max_height = 2160;
> +	drm->mode_config.max_width = 16384;
> +	drm->mode_config.max_height = 8192;
>  	drm->mode_config.funcs = &meson_mode_config_funcs;
>  
>  	/* Hardware Initialization */
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
> The mode_config max_width/max_height determines the maximum framebuffer
> size the pixel reader can handle. But the values were set thinking they
> were determining the maximum screen dimensions.
> 
> This patch changes the values to the maximum height/width the CANVAS block
> can handle rounded to some coherent values.
> 
> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

It's both. Grep for all the callers of ->fill_modes and you'll see that
this limit is also used to filter max screen sizes.

If you want to change this, then I think we need a new
mode_config.fb_max_width/height, which if non-zero, would extend the limit
for fbs.

There's also the problem that if you extend this for fbs, then there's no
check anymore in the atomic_commit paths (or legacy modeset), so that
needs to be addressed somehow too.

Bunch of igt to make sure we're not missing anything would be sweet on
top, e.g. e.g. trying to set a mode over the limit and making sure it
fails.

Cheers, Daniel

> ---
>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index d344312..2e29968 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  		goto free_drm;
>  
>  	drm_mode_config_init(drm);
> -	drm->mode_config.max_width = 3840;
> -	drm->mode_config.max_height = 2160;
> +	drm->mode_config.max_width = 16384;
> +	drm->mode_config.max_height = 8192;
>  	drm->mode_config.funcs = &meson_mode_config_funcs;
>  
>  	/* Hardware Initialization */
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 04/10/2018 12:09, Daniel Vetter wrote:
> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
>> The mode_config max_width/max_height determines the maximum framebuffer
>> size the pixel reader can handle. But the values were set thinking they
>> were determining the maximum screen dimensions.
>>
>> This patch changes the values to the maximum height/width the CANVAS block
>> can handle rounded to some coherent values.
>>
>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> It's both. Grep for all the callers of ->fill_modes and you'll see that
> this limit is also used to filter max screen sizes.
> 
> If you want to change this, then I think we need a new
> mode_config.fb_max_width/height, which if non-zero, would extend the limit
> for fbs.
> 
> There's also the problem that if you extend this for fbs, then there's no
> check anymore in the atomic_commit paths (or legacy modeset), so that
> needs to be addressed somehow too.

What about adding optionals mode_config.fb_max_width/height and update
drm_internal_framebuffer_create() to use these if non-0 or fallback
to the mode_config max_width/max_height.

Neil

> 
> Bunch of igt to make sure we're not missing anything would be sweet on
> top, e.g. e.g. trying to set a mode over the limit and making sure it
> fails.
> 
> Cheers, Daniel
> 
>> ---
>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>> index d344312..2e29968 100644
>> --- a/drivers/gpu/drm/meson/meson_drv.c
>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>  		goto free_drm;
>>  
>>  	drm_mode_config_init(drm);
>> -	drm->mode_config.max_width = 3840;
>> -	drm->mode_config.max_height = 2160;
>> +	drm->mode_config.max_width = 16384;
>> +	drm->mode_config.max_height = 8192;
>>  	drm->mode_config.funcs = &meson_mode_config_funcs;
>>  
>>  	/* Hardware Initialization */
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 04/10/2018 12:09, Daniel Vetter wrote:
> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
>> The mode_config max_width/max_height determines the maximum framebuffer
>> size the pixel reader can handle. But the values were set thinking they
>> were determining the maximum screen dimensions.
>>
>> This patch changes the values to the maximum height/width the CANVAS block
>> can handle rounded to some coherent values.
>>
>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> It's both. Grep for all the callers of ->fill_modes and you'll see that
> this limit is also used to filter max screen sizes.
> 
> If you want to change this, then I think we need a new
> mode_config.fb_max_width/height, which if non-zero, would extend the limit
> for fbs.
> 
> There's also the problem that if you extend this for fbs, then there's no
> check anymore in the atomic_commit paths (or legacy modeset), so that
> needs to be addressed somehow too.

What about adding optionals mode_config.fb_max_width/height and update
drm_internal_framebuffer_create() to use these if non-0 or fallback
to the mode_config max_width/max_height.

Neil

> 
> Bunch of igt to make sure we're not missing anything would be sweet on
> top, e.g. e.g. trying to set a mode over the limit and making sure it
> fails.
> 
> Cheers, Daniel
> 
>> ---
>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>> index d344312..2e29968 100644
>> --- a/drivers/gpu/drm/meson/meson_drv.c
>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>  		goto free_drm;
>>  
>>  	drm_mode_config_init(drm);
>> -	drm->mode_config.max_width = 3840;
>> -	drm->mode_config.max_height = 2160;
>> +	drm->mode_config.max_width = 16384;
>> +	drm->mode_config.max_height = 8192;
>>  	drm->mode_config.funcs = &meson_mode_config_funcs;
>>  
>>  	/* Hardware Initialization */
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 04/10/2018 12:09, Daniel Vetter wrote:
> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
>> The mode_config max_width/max_height determines the maximum framebuffer
>> size the pixel reader can handle. But the values were set thinking they
>> were determining the maximum screen dimensions.
>>
>> This patch changes the values to the maximum height/width the CANVAS block
>> can handle rounded to some coherent values.
>>
>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> It's both. Grep for all the callers of ->fill_modes and you'll see that
> this limit is also used to filter max screen sizes.
> 
> If you want to change this, then I think we need a new
> mode_config.fb_max_width/height, which if non-zero, would extend the limit
> for fbs.
> 
> There's also the problem that if you extend this for fbs, then there's no
> check anymore in the atomic_commit paths (or legacy modeset), so that
> needs to be addressed somehow too.

What about adding optionals mode_config.fb_max_width/height and update
drm_internal_framebuffer_create() to use these if non-0 or fallback
to the mode_config max_width/max_height.

Neil

> 
> Bunch of igt to make sure we're not missing anything would be sweet on
> top, e.g. e.g. trying to set a mode over the limit and making sure it
> fails.
> 
> Cheers, Daniel
> 
>> ---
>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>> index d344312..2e29968 100644
>> --- a/drivers/gpu/drm/meson/meson_drv.c
>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>  		goto free_drm;
>>  
>>  	drm_mode_config_init(drm);
>> -	drm->mode_config.max_width = 3840;
>> -	drm->mode_config.max_height = 2160;
>> +	drm->mode_config.max_width = 16384;
>> +	drm->mode_config.max_height = 8192;
>>  	drm->mode_config.funcs = &meson_mode_config_funcs;
>>  
>>  	/* Hardware Initialization */
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 04/10/2018 12:09, Daniel Vetter wrote:
> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
>> The mode_config max_width/max_height determines the maximum framebuffer
>> size the pixel reader can handle. But the values were set thinking they
>> were determining the maximum screen dimensions.
>>
>> This patch changes the values to the maximum height/width the CANVAS block
>> can handle rounded to some coherent values.
>>
>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> It's both. Grep for all the callers of ->fill_modes and you'll see that
> this limit is also used to filter max screen sizes.
> 
> If you want to change this, then I think we need a new
> mode_config.fb_max_width/height, which if non-zero, would extend the limit
> for fbs.
> 
> There's also the problem that if you extend this for fbs, then there's no
> check anymore in the atomic_commit paths (or legacy modeset), so that
> needs to be addressed somehow too.

What about adding optionals mode_config.fb_max_width/height and update
drm_internal_framebuffer_create() to use these if non-0 or fallback
to the mode_config max_width/max_height.

Neil

> 
> Bunch of igt to make sure we're not missing anything would be sweet on
> top, e.g. e.g. trying to set a mode over the limit and making sure it
> fails.
> 
> Cheers, Daniel
> 
>> ---
>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>> index d344312..2e29968 100644
>> --- a/drivers/gpu/drm/meson/meson_drv.c
>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>  		goto free_drm;
>>  
>>  	drm_mode_config_init(drm);
>> -	drm->mode_config.max_width = 3840;
>> -	drm->mode_config.max_height = 2160;
>> +	drm->mode_config.max_width = 16384;
>> +	drm->mode_config.max_height = 8192;
>>  	drm->mode_config.funcs = &meson_mode_config_funcs;
>>  
>>  	/* Hardware Initialization */
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

[PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Thu, Oct 4, 2018 at 5:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 04/10/2018 12:09, Daniel Vetter wrote:
> > On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
> >> The mode_config max_width/max_height determines the maximum framebuffer
> >> size the pixel reader can handle. But the values were set thinking they
> >> were determining the maximum screen dimensions.
> >>
> >> This patch changes the values to the maximum height/width the CANVAS block
> >> can handle rounded to some coherent values.
> >>
> >> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >
> > It's both. Grep for all the callers of ->fill_modes and you'll see that
> > this limit is also used to filter max screen sizes.
> >
> > If you want to change this, then I think we need a new
> > mode_config.fb_max_width/height, which if non-zero, would extend the limit
> > for fbs.
> >
> > There's also the problem that if you extend this for fbs, then there's no
> > check anymore in the atomic_commit paths (or legacy modeset), so that
> > needs to be addressed somehow too.
>
> What about adding optionals mode_config.fb_max_width/height and update
> drm_internal_framebuffer_create() to use these if non-0 or fallback
> to the mode_config max_width/max_height.

That's what I meant. Except you also need to then fix the gap you've
opened in atomic_check, and validate the mode size against
mode_config.max_width/height.
-Daniel

>
> Neil
>
> >
> > Bunch of igt to make sure we're not missing anything would be sweet on
> > top, e.g. e.g. trying to set a mode over the limit and making sure it
> > fails.
> >
> > Cheers, Daniel
> >
> >> ---
> >>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> >> index d344312..2e29968 100644
> >> --- a/drivers/gpu/drm/meson/meson_drv.c
> >> +++ b/drivers/gpu/drm/meson/meson_drv.c
> >> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> >>              goto free_drm;
> >>
> >>      drm_mode_config_init(drm);
> >> -    drm->mode_config.max_width = 3840;
> >> -    drm->mode_config.max_height = 2160;
> >> +    drm->mode_config.max_width = 16384;
> >> +    drm->mode_config.max_height = 8192;
> >>      drm->mode_config.funcs = &meson_mode_config_funcs;
> >>
> >>      /* Hardware Initialization */
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Thu, Oct 4, 2018 at 5:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 04/10/2018 12:09, Daniel Vetter wrote:
> > On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
> >> The mode_config max_width/max_height determines the maximum framebuffer
> >> size the pixel reader can handle. But the values were set thinking they
> >> were determining the maximum screen dimensions.
> >>
> >> This patch changes the values to the maximum height/width the CANVAS block
> >> can handle rounded to some coherent values.
> >>
> >> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >
> > It's both. Grep for all the callers of ->fill_modes and you'll see that
> > this limit is also used to filter max screen sizes.
> >
> > If you want to change this, then I think we need a new
> > mode_config.fb_max_width/height, which if non-zero, would extend the limit
> > for fbs.
> >
> > There's also the problem that if you extend this for fbs, then there's no
> > check anymore in the atomic_commit paths (or legacy modeset), so that
> > needs to be addressed somehow too.
>
> What about adding optionals mode_config.fb_max_width/height and update
> drm_internal_framebuffer_create() to use these if non-0 or fallback
> to the mode_config max_width/max_height.

That's what I meant. Except you also need to then fix the gap you've
opened in atomic_check, and validate the mode size against
mode_config.max_width/height.
-Daniel

>
> Neil
>
> >
> > Bunch of igt to make sure we're not missing anything would be sweet on
> > top, e.g. e.g. trying to set a mode over the limit and making sure it
> > fails.
> >
> > Cheers, Daniel
> >
> >> ---
> >>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> >> index d344312..2e29968 100644
> >> --- a/drivers/gpu/drm/meson/meson_drv.c
> >> +++ b/drivers/gpu/drm/meson/meson_drv.c
> >> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> >>              goto free_drm;
> >>
> >>      drm_mode_config_init(drm);
> >> -    drm->mode_config.max_width = 3840;
> >> -    drm->mode_config.max_height = 2160;
> >> +    drm->mode_config.max_width = 16384;
> >> +    drm->mode_config.max_height = 8192;
> >>      drm->mode_config.funcs = &meson_mode_config_funcs;
> >>
> >>      /* Hardware Initialization */
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Thu, Oct 4, 2018 at 5:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 04/10/2018 12:09, Daniel Vetter wrote:
> > On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
> >> The mode_config max_width/max_height determines the maximum framebuffer
> >> size the pixel reader can handle. But the values were set thinking they
> >> were determining the maximum screen dimensions.
> >>
> >> This patch changes the values to the maximum height/width the CANVAS block
> >> can handle rounded to some coherent values.
> >>
> >> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >
> > It's both. Grep for all the callers of ->fill_modes and you'll see that
> > this limit is also used to filter max screen sizes.
> >
> > If you want to change this, then I think we need a new
> > mode_config.fb_max_width/height, which if non-zero, would extend the limit
> > for fbs.
> >
> > There's also the problem that if you extend this for fbs, then there's no
> > check anymore in the atomic_commit paths (or legacy modeset), so that
> > needs to be addressed somehow too.
>
> What about adding optionals mode_config.fb_max_width/height and update
> drm_internal_framebuffer_create() to use these if non-0 or fallback
> to the mode_config max_width/max_height.

That's what I meant. Except you also need to then fix the gap you've
opened in atomic_check, and validate the mode size against
mode_config.max_width/height.
-Daniel

>
> Neil
>
> >
> > Bunch of igt to make sure we're not missing anything would be sweet on
> > top, e.g. e.g. trying to set a mode over the limit and making sure it
> > fails.
> >
> > Cheers, Daniel
> >
> >> ---
> >>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> >> index d344312..2e29968 100644
> >> --- a/drivers/gpu/drm/meson/meson_drv.c
> >> +++ b/drivers/gpu/drm/meson/meson_drv.c
> >> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> >>              goto free_drm;
> >>
> >>      drm_mode_config_init(drm);
> >> -    drm->mode_config.max_width = 3840;
> >> -    drm->mode_config.max_height = 2160;
> >> +    drm->mode_config.max_width = 16384;
> >> +    drm->mode_config.max_height = 8192;
> >>      drm->mode_config.funcs = &meson_mode_config_funcs;
> >>
> >>      /* Hardware Initialization */
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 04/10/2018 20:10, Daniel Vetter wrote:
> On Thu, Oct 4, 2018 at 5:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> On 04/10/2018 12:09, Daniel Vetter wrote:
>>> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
>>>> The mode_config max_width/max_height determines the maximum framebuffer
>>>> size the pixel reader can handle. But the values were set thinking they
>>>> were determining the maximum screen dimensions.
>>>>
>>>> This patch changes the values to the maximum height/width the CANVAS block
>>>> can handle rounded to some coherent values.
>>>>
>>>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>
>>> It's both. Grep for all the callers of ->fill_modes and you'll see that
>>> this limit is also used to filter max screen sizes.
>>>
>>> If you want to change this, then I think we need a new
>>> mode_config.fb_max_width/height, which if non-zero, would extend the limit
>>> for fbs.
>>>
>>> There's also the problem that if you extend this for fbs, then there's no
>>> check anymore in the atomic_commit paths (or legacy modeset), so that
>>> needs to be addressed somehow too.
>>
>> What about adding optionals mode_config.fb_max_width/height and update
>> drm_internal_framebuffer_create() to use these if non-0 or fallback
>> to the mode_config max_width/max_height.
> 
> That's what I meant. Except you also need to then fix the gap you've
> opened in atomic_check, and validate the mode size against
> mode_config.max_width/height.

OK, won't this be enough ?
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
  * @min_height: minimum fb pixel height on this device
  * @max_width: maximum fb pixel width on this device
  * @max_height: maximum fb pixel height on this device
+ * @max_fb_width: maximum fb buffer width if differs from max_width
+ * @max_fb_height: maximum fb buffer height if differs from max_height
  * @funcs: core driver provided mode setting functions
  * @fb_base: base address of the framebuffer
  * @poll_enabled: track polling support for this device
@@ -508,6 +510,7 @@ struct drm_mode_config {

        int min_width, min_height;
        int max_width, max_height;
+       int max_fb_width, max_fb_height;
        const struct drm_mode_config_funcs *funcs;
        resource_size_t fb_base;

--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
                return ERR_PTR(-EINVAL);
        }

-       if ((config->min_width > r->width) || (r->width > config->max_width)) {
+       if ((config->min_width > r->width) ||
+           (!config->max_fb_width && r->width > config->max_width) ||
+           (config->max_fb_width && r->width > config->max_fb_width)) {
                DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
-                         r->width, config->min_width, config->max_width);
+                         r->width, config->min_width, config->max_fb_width ?
+                         config->max_fb_width : config->max_width);
                return ERR_PTR(-EINVAL);
        }
-       if ((config->min_height > r->height) || (r->height > config->max_height)) {
+       if ((config->min_height > r->height) ||
+           (!config->max_fb_height && r->height > config->max_height) ||
+           (config->max_fb_height && r->height > config->max_fb_height)) {
                DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
-                         r->height, config->min_height, config->max_height);
+                         r->height, config->min_height, config->max_fb_height ?
+                         config->max_fb_height : config->max_height);
                return ERR_PTR(-EINVAL);
        }

and in the driver :

+       drm->mode_config.max_width = 4096;
+       drm->mode_config.max_height = 3840;
+       drm->mode_config.max_fb_width = 16384;
+       drm->mode_config.max_fb_height = 8192;

With this I leave the mode filtering intact.

Neil


> -Daniel
> 
>>
>> Neil
>>
>>>
>>> Bunch of igt to make sure we're not missing anything would be sweet on
>>> top, e.g. e.g. trying to set a mode over the limit and making sure it
>>> fails.
>>>
>>> Cheers, Daniel
>>>
>>>> ---
>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>>>> index d344312..2e29968 100644
>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>>>              goto free_drm;
>>>>
>>>>      drm_mode_config_init(drm);
>>>> -    drm->mode_config.max_width = 3840;
>>>> -    drm->mode_config.max_height = 2160;
>>>> +    drm->mode_config.max_width = 16384;
>>>> +    drm->mode_config.max_height = 8192;
>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>
>>>>      /* Hardware Initialization */
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 

Re: [PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 04/10/2018 20:10, Daniel Vetter wrote:
> On Thu, Oct 4, 2018 at 5:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> On 04/10/2018 12:09, Daniel Vetter wrote:
>>> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
>>>> The mode_config max_width/max_height determines the maximum framebuffer
>>>> size the pixel reader can handle. But the values were set thinking they
>>>> were determining the maximum screen dimensions.
>>>>
>>>> This patch changes the values to the maximum height/width the CANVAS block
>>>> can handle rounded to some coherent values.
>>>>
>>>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>
>>> It's both. Grep for all the callers of ->fill_modes and you'll see that
>>> this limit is also used to filter max screen sizes.
>>>
>>> If you want to change this, then I think we need a new
>>> mode_config.fb_max_width/height, which if non-zero, would extend the limit
>>> for fbs.
>>>
>>> There's also the problem that if you extend this for fbs, then there's no
>>> check anymore in the atomic_commit paths (or legacy modeset), so that
>>> needs to be addressed somehow too.
>>
>> What about adding optionals mode_config.fb_max_width/height and update
>> drm_internal_framebuffer_create() to use these if non-0 or fallback
>> to the mode_config max_width/max_height.
> 
> That's what I meant. Except you also need to then fix the gap you've
> opened in atomic_check, and validate the mode size against
> mode_config.max_width/height.

OK, won't this be enough ?
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
  * @min_height: minimum fb pixel height on this device
  * @max_width: maximum fb pixel width on this device
  * @max_height: maximum fb pixel height on this device
+ * @max_fb_width: maximum fb buffer width if differs from max_width
+ * @max_fb_height: maximum fb buffer height if differs from max_height
  * @funcs: core driver provided mode setting functions
  * @fb_base: base address of the framebuffer
  * @poll_enabled: track polling support for this device
@@ -508,6 +510,7 @@ struct drm_mode_config {

        int min_width, min_height;
        int max_width, max_height;
+       int max_fb_width, max_fb_height;
        const struct drm_mode_config_funcs *funcs;
        resource_size_t fb_base;

--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
                return ERR_PTR(-EINVAL);
        }

-       if ((config->min_width > r->width) || (r->width > config->max_width)) {
+       if ((config->min_width > r->width) ||
+           (!config->max_fb_width && r->width > config->max_width) ||
+           (config->max_fb_width && r->width > config->max_fb_width)) {
                DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
-                         r->width, config->min_width, config->max_width);
+                         r->width, config->min_width, config->max_fb_width ?
+                         config->max_fb_width : config->max_width);
                return ERR_PTR(-EINVAL);
        }
-       if ((config->min_height > r->height) || (r->height > config->max_height)) {
+       if ((config->min_height > r->height) ||
+           (!config->max_fb_height && r->height > config->max_height) ||
+           (config->max_fb_height && r->height > config->max_fb_height)) {
                DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
-                         r->height, config->min_height, config->max_height);
+                         r->height, config->min_height, config->max_fb_height ?
+                         config->max_fb_height : config->max_height);
                return ERR_PTR(-EINVAL);
        }

and in the driver :

+       drm->mode_config.max_width = 4096;
+       drm->mode_config.max_height = 3840;
+       drm->mode_config.max_fb_width = 16384;
+       drm->mode_config.max_fb_height = 8192;

With this I leave the mode filtering intact.

Neil


> -Daniel
> 
>>
>> Neil
>>
>>>
>>> Bunch of igt to make sure we're not missing anything would be sweet on
>>> top, e.g. e.g. trying to set a mode over the limit and making sure it
>>> fails.
>>>
>>> Cheers, Daniel
>>>
>>>> ---
>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>>>> index d344312..2e29968 100644
>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>>>              goto free_drm;
>>>>
>>>>      drm_mode_config_init(drm);
>>>> -    drm->mode_config.max_width = 3840;
>>>> -    drm->mode_config.max_height = 2160;
>>>> +    drm->mode_config.max_width = 16384;
>>>> +    drm->mode_config.max_height = 8192;
>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>
>>>>      /* Hardware Initialization */
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 04/10/2018 20:10, Daniel Vetter wrote:
> On Thu, Oct 4, 2018 at 5:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> On 04/10/2018 12:09, Daniel Vetter wrote:
>>> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
>>>> The mode_config max_width/max_height determines the maximum framebuffer
>>>> size the pixel reader can handle. But the values were set thinking they
>>>> were determining the maximum screen dimensions.
>>>>
>>>> This patch changes the values to the maximum height/width the CANVAS block
>>>> can handle rounded to some coherent values.
>>>>
>>>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>
>>> It's both. Grep for all the callers of ->fill_modes and you'll see that
>>> this limit is also used to filter max screen sizes.
>>>
>>> If you want to change this, then I think we need a new
>>> mode_config.fb_max_width/height, which if non-zero, would extend the limit
>>> for fbs.
>>>
>>> There's also the problem that if you extend this for fbs, then there's no
>>> check anymore in the atomic_commit paths (or legacy modeset), so that
>>> needs to be addressed somehow too.
>>
>> What about adding optionals mode_config.fb_max_width/height and update
>> drm_internal_framebuffer_create() to use these if non-0 or fallback
>> to the mode_config max_width/max_height.
> 
> That's what I meant. Except you also need to then fix the gap you've
> opened in atomic_check, and validate the mode size against
> mode_config.max_width/height.

OK, won't this be enough ?
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
  * @min_height: minimum fb pixel height on this device
  * @max_width: maximum fb pixel width on this device
  * @max_height: maximum fb pixel height on this device
+ * @max_fb_width: maximum fb buffer width if differs from max_width
+ * @max_fb_height: maximum fb buffer height if differs from max_height
  * @funcs: core driver provided mode setting functions
  * @fb_base: base address of the framebuffer
  * @poll_enabled: track polling support for this device
@@ -508,6 +510,7 @@ struct drm_mode_config {

        int min_width, min_height;
        int max_width, max_height;
+       int max_fb_width, max_fb_height;
        const struct drm_mode_config_funcs *funcs;
        resource_size_t fb_base;

--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
                return ERR_PTR(-EINVAL);
        }

-       if ((config->min_width > r->width) || (r->width > config->max_width)) {
+       if ((config->min_width > r->width) ||
+           (!config->max_fb_width && r->width > config->max_width) ||
+           (config->max_fb_width && r->width > config->max_fb_width)) {
                DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
-                         r->width, config->min_width, config->max_width);
+                         r->width, config->min_width, config->max_fb_width ?
+                         config->max_fb_width : config->max_width);
                return ERR_PTR(-EINVAL);
        }
-       if ((config->min_height > r->height) || (r->height > config->max_height)) {
+       if ((config->min_height > r->height) ||
+           (!config->max_fb_height && r->height > config->max_height) ||
+           (config->max_fb_height && r->height > config->max_fb_height)) {
                DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
-                         r->height, config->min_height, config->max_height);
+                         r->height, config->min_height, config->max_fb_height ?
+                         config->max_fb_height : config->max_height);
                return ERR_PTR(-EINVAL);
        }

and in the driver :

+       drm->mode_config.max_width = 4096;
+       drm->mode_config.max_height = 3840;
+       drm->mode_config.max_fb_width = 16384;
+       drm->mode_config.max_fb_height = 8192;

With this I leave the mode filtering intact.

Neil


> -Daniel
> 
>>
>> Neil
>>
>>>
>>> Bunch of igt to make sure we're not missing anything would be sweet on
>>> top, e.g. e.g. trying to set a mode over the limit and making sure it
>>> fails.
>>>
>>> Cheers, Daniel
>>>
>>>> ---
>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>>>> index d344312..2e29968 100644
>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>>>              goto free_drm;
>>>>
>>>>      drm_mode_config_init(drm);
>>>> -    drm->mode_config.max_width = 3840;
>>>> -    drm->mode_config.max_height = 2160;
>>>> +    drm->mode_config.max_width = 16384;
>>>> +    drm->mode_config.max_height = 8192;
>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>
>>>>      /* Hardware Initialization */
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 

[PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 04/10/2018 20:10, Daniel Vetter wrote:
> > On Thu, Oct 4, 2018 at 5:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >>
> >> On 04/10/2018 12:09, Daniel Vetter wrote:
> >>> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
> >>>> The mode_config max_width/max_height determines the maximum framebuffer
> >>>> size the pixel reader can handle. But the values were set thinking they
> >>>> were determining the maximum screen dimensions.
> >>>>
> >>>> This patch changes the values to the maximum height/width the CANVAS block
> >>>> can handle rounded to some coherent values.
> >>>>
> >>>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
> >>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >>>
> >>> It's both. Grep for all the callers of ->fill_modes and you'll see that
> >>> this limit is also used to filter max screen sizes.
> >>>
> >>> If you want to change this, then I think we need a new
> >>> mode_config.fb_max_width/height, which if non-zero, would extend the limit
> >>> for fbs.
> >>>
> >>> There's also the problem that if you extend this for fbs, then there's no
> >>> check anymore in the atomic_commit paths (or legacy modeset), so that
> >>> needs to be addressed somehow too.
> >>
> >> What about adding optionals mode_config.fb_max_width/height and update
> >> drm_internal_framebuffer_create() to use these if non-0 or fallback
> >> to the mode_config max_width/max_height.
> >
> > That's what I meant. Except you also need to then fix the gap you've
> > opened in atomic_check, and validate the mode size against
> > mode_config.max_width/height.
>
> OK, won't this be enough ?
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>   * @min_height: minimum fb pixel height on this device
>   * @max_width: maximum fb pixel width on this device
>   * @max_height: maximum fb pixel height on this device
> + * @max_fb_width: maximum fb buffer width if differs from max_width
> + * @max_fb_height: maximum fb buffer height if differs from max_height
>   * @funcs: core driver provided mode setting functions
>   * @fb_base: base address of the framebuffer
>   * @poll_enabled: track polling support for this device
> @@ -508,6 +510,7 @@ struct drm_mode_config {
>
>         int min_width, min_height;
>         int max_width, max_height;
> +       int max_fb_width, max_fb_height;
>         const struct drm_mode_config_funcs *funcs;
>         resource_size_t fb_base;
>
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
>                 return ERR_PTR(-EINVAL);
>         }
>
> -       if ((config->min_width > r->width) || (r->width > config->max_width)) {
> +       if ((config->min_width > r->width) ||
> +           (!config->max_fb_width && r->width > config->max_width) ||
> +           (config->max_fb_width && r->width > config->max_fb_width)) {
>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
> -                         r->width, config->min_width, config->max_width);
> +                         r->width, config->min_width, config->max_fb_width ?
> +                         config->max_fb_width : config->max_width);
>                 return ERR_PTR(-EINVAL);
>         }
> -       if ((config->min_height > r->height) || (r->height > config->max_height)) {
> +       if ((config->min_height > r->height) ||
> +           (!config->max_fb_height && r->height > config->max_height) ||
> +           (config->max_fb_height && r->height > config->max_fb_height)) {
>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
> -                         r->height, config->min_height, config->max_height);
> +                         r->height, config->min_height, config->max_fb_height ?
> +                         config->max_fb_height : config->max_height);
>                 return ERR_PTR(-EINVAL);
>         }
>
> and in the driver :
>
> +       drm->mode_config.max_width = 4096;
> +       drm->mode_config.max_height = 3840;
> +       drm->mode_config.max_fb_width = 16384;
> +       drm->mode_config.max_fb_height = 8192;
>
> With this I leave the mode filtering intact.

Not enough. See
https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
and scroll down to mode_valid. You need to filter modes both in the
detect paths, and the atomic_check paths.

Detect is explicitly filtered out, but atomic_check was only
implicitly filtered, through the max fb size checks. Ok, you could
light up a mode that's bigger than max fb, but in practice, no
userspace ever did that. But with your code we're missing crucial
validation now, and userspace could fall over that. What I think we
need is to add mode filter against mode_config.max_width/height in
drm_atomic_helper_check_modeset(). Probably best to stuff that into
the mode_valid() function.

Cheers, Daniel
>
> Neil
>
>
> > -Daniel
> >
> >>
> >> Neil
> >>
> >>>
> >>> Bunch of igt to make sure we're not missing anything would be sweet on
> >>> top, e.g. e.g. trying to set a mode over the limit and making sure it
> >>> fails.
> >>>
> >>> Cheers, Daniel
> >>>
> >>>> ---
> >>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> >>>> index d344312..2e29968 100644
> >>>> --- a/drivers/gpu/drm/meson/meson_drv.c
> >>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
> >>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> >>>>              goto free_drm;
> >>>>
> >>>>      drm_mode_config_init(drm);
> >>>> -    drm->mode_config.max_width = 3840;
> >>>> -    drm->mode_config.max_height = 2160;
> >>>> +    drm->mode_config.max_width = 16384;
> >>>> +    drm->mode_config.max_height = 8192;
> >>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
> >>>>
> >>>>      /* Hardware Initialization */
> >>>> --
> >>>> 2.7.4
> >>>>
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel at lists.freedesktop.org
> >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 04/10/2018 20:10, Daniel Vetter wrote:
> > On Thu, Oct 4, 2018 at 5:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >>
> >> On 04/10/2018 12:09, Daniel Vetter wrote:
> >>> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
> >>>> The mode_config max_width/max_height determines the maximum framebuffer
> >>>> size the pixel reader can handle. But the values were set thinking they
> >>>> were determining the maximum screen dimensions.
> >>>>
> >>>> This patch changes the values to the maximum height/width the CANVAS block
> >>>> can handle rounded to some coherent values.
> >>>>
> >>>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
> >>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >>>
> >>> It's both. Grep for all the callers of ->fill_modes and you'll see that
> >>> this limit is also used to filter max screen sizes.
> >>>
> >>> If you want to change this, then I think we need a new
> >>> mode_config.fb_max_width/height, which if non-zero, would extend the limit
> >>> for fbs.
> >>>
> >>> There's also the problem that if you extend this for fbs, then there's no
> >>> check anymore in the atomic_commit paths (or legacy modeset), so that
> >>> needs to be addressed somehow too.
> >>
> >> What about adding optionals mode_config.fb_max_width/height and update
> >> drm_internal_framebuffer_create() to use these if non-0 or fallback
> >> to the mode_config max_width/max_height.
> >
> > That's what I meant. Except you also need to then fix the gap you've
> > opened in atomic_check, and validate the mode size against
> > mode_config.max_width/height.
>
> OK, won't this be enough ?
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>   * @min_height: minimum fb pixel height on this device
>   * @max_width: maximum fb pixel width on this device
>   * @max_height: maximum fb pixel height on this device
> + * @max_fb_width: maximum fb buffer width if differs from max_width
> + * @max_fb_height: maximum fb buffer height if differs from max_height
>   * @funcs: core driver provided mode setting functions
>   * @fb_base: base address of the framebuffer
>   * @poll_enabled: track polling support for this device
> @@ -508,6 +510,7 @@ struct drm_mode_config {
>
>         int min_width, min_height;
>         int max_width, max_height;
> +       int max_fb_width, max_fb_height;
>         const struct drm_mode_config_funcs *funcs;
>         resource_size_t fb_base;
>
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
>                 return ERR_PTR(-EINVAL);
>         }
>
> -       if ((config->min_width > r->width) || (r->width > config->max_width)) {
> +       if ((config->min_width > r->width) ||
> +           (!config->max_fb_width && r->width > config->max_width) ||
> +           (config->max_fb_width && r->width > config->max_fb_width)) {
>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
> -                         r->width, config->min_width, config->max_width);
> +                         r->width, config->min_width, config->max_fb_width ?
> +                         config->max_fb_width : config->max_width);
>                 return ERR_PTR(-EINVAL);
>         }
> -       if ((config->min_height > r->height) || (r->height > config->max_height)) {
> +       if ((config->min_height > r->height) ||
> +           (!config->max_fb_height && r->height > config->max_height) ||
> +           (config->max_fb_height && r->height > config->max_fb_height)) {
>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
> -                         r->height, config->min_height, config->max_height);
> +                         r->height, config->min_height, config->max_fb_height ?
> +                         config->max_fb_height : config->max_height);
>                 return ERR_PTR(-EINVAL);
>         }
>
> and in the driver :
>
> +       drm->mode_config.max_width = 4096;
> +       drm->mode_config.max_height = 3840;
> +       drm->mode_config.max_fb_width = 16384;
> +       drm->mode_config.max_fb_height = 8192;
>
> With this I leave the mode filtering intact.

Not enough. See
https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
and scroll down to mode_valid. You need to filter modes both in the
detect paths, and the atomic_check paths.

Detect is explicitly filtered out, but atomic_check was only
implicitly filtered, through the max fb size checks. Ok, you could
light up a mode that's bigger than max fb, but in practice, no
userspace ever did that. But with your code we're missing crucial
validation now, and userspace could fall over that. What I think we
need is to add mode filter against mode_config.max_width/height in
drm_atomic_helper_check_modeset(). Probably best to stuff that into
the mode_valid() function.

Cheers, Daniel
>
> Neil
>
>
> > -Daniel
> >
> >>
> >> Neil
> >>
> >>>
> >>> Bunch of igt to make sure we're not missing anything would be sweet on
> >>> top, e.g. e.g. trying to set a mode over the limit and making sure it
> >>> fails.
> >>>
> >>> Cheers, Daniel
> >>>
> >>>> ---
> >>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> >>>> index d344312..2e29968 100644
> >>>> --- a/drivers/gpu/drm/meson/meson_drv.c
> >>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
> >>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> >>>>              goto free_drm;
> >>>>
> >>>>      drm_mode_config_init(drm);
> >>>> -    drm->mode_config.max_width = 3840;
> >>>> -    drm->mode_config.max_height = 2160;
> >>>> +    drm->mode_config.max_width = 16384;
> >>>> +    drm->mode_config.max_height = 8192;
> >>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
> >>>>
> >>>>      /* Hardware Initialization */
> >>>> --
> >>>> 2.7.4
> >>>>
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel@lists.freedesktop.org
> >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 04/10/2018 20:10, Daniel Vetter wrote:
> > On Thu, Oct 4, 2018 at 5:05 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >>
> >> On 04/10/2018 12:09, Daniel Vetter wrote:
> >>> On Thu, Oct 04, 2018 at 10:42:43AM +0200, Neil Armstrong wrote:
> >>>> The mode_config max_width/max_height determines the maximum framebuffer
> >>>> size the pixel reader can handle. But the values were set thinking they
> >>>> were determining the maximum screen dimensions.
> >>>>
> >>>> This patch changes the values to the maximum height/width the CANVAS block
> >>>> can handle rounded to some coherent values.
> >>>>
> >>>> Fixes: a41e82e6c457 ("drm/meson: Add support for components")
> >>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >>>
> >>> It's both. Grep for all the callers of ->fill_modes and you'll see that
> >>> this limit is also used to filter max screen sizes.
> >>>
> >>> If you want to change this, then I think we need a new
> >>> mode_config.fb_max_width/height, which if non-zero, would extend the limit
> >>> for fbs.
> >>>
> >>> There's also the problem that if you extend this for fbs, then there's no
> >>> check anymore in the atomic_commit paths (or legacy modeset), so that
> >>> needs to be addressed somehow too.
> >>
> >> What about adding optionals mode_config.fb_max_width/height and update
> >> drm_internal_framebuffer_create() to use these if non-0 or fallback
> >> to the mode_config max_width/max_height.
> >
> > That's what I meant. Except you also need to then fix the gap you've
> > opened in atomic_check, and validate the mode size against
> > mode_config.max_width/height.
>
> OK, won't this be enough ?
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>   * @min_height: minimum fb pixel height on this device
>   * @max_width: maximum fb pixel width on this device
>   * @max_height: maximum fb pixel height on this device
> + * @max_fb_width: maximum fb buffer width if differs from max_width
> + * @max_fb_height: maximum fb buffer height if differs from max_height
>   * @funcs: core driver provided mode setting functions
>   * @fb_base: base address of the framebuffer
>   * @poll_enabled: track polling support for this device
> @@ -508,6 +510,7 @@ struct drm_mode_config {
>
>         int min_width, min_height;
>         int max_width, max_height;
> +       int max_fb_width, max_fb_height;
>         const struct drm_mode_config_funcs *funcs;
>         resource_size_t fb_base;
>
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
>                 return ERR_PTR(-EINVAL);
>         }
>
> -       if ((config->min_width > r->width) || (r->width > config->max_width)) {
> +       if ((config->min_width > r->width) ||
> +           (!config->max_fb_width && r->width > config->max_width) ||
> +           (config->max_fb_width && r->width > config->max_fb_width)) {
>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
> -                         r->width, config->min_width, config->max_width);
> +                         r->width, config->min_width, config->max_fb_width ?
> +                         config->max_fb_width : config->max_width);
>                 return ERR_PTR(-EINVAL);
>         }
> -       if ((config->min_height > r->height) || (r->height > config->max_height)) {
> +       if ((config->min_height > r->height) ||
> +           (!config->max_fb_height && r->height > config->max_height) ||
> +           (config->max_fb_height && r->height > config->max_fb_height)) {
>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
> -                         r->height, config->min_height, config->max_height);
> +                         r->height, config->min_height, config->max_fb_height ?
> +                         config->max_fb_height : config->max_height);
>                 return ERR_PTR(-EINVAL);
>         }
>
> and in the driver :
>
> +       drm->mode_config.max_width = 4096;
> +       drm->mode_config.max_height = 3840;
> +       drm->mode_config.max_fb_width = 16384;
> +       drm->mode_config.max_fb_height = 8192;
>
> With this I leave the mode filtering intact.

Not enough. See
https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
and scroll down to mode_valid. You need to filter modes both in the
detect paths, and the atomic_check paths.

Detect is explicitly filtered out, but atomic_check was only
implicitly filtered, through the max fb size checks. Ok, you could
light up a mode that's bigger than max fb, but in practice, no
userspace ever did that. But with your code we're missing crucial
validation now, and userspace could fall over that. What I think we
need is to add mode filter against mode_config.max_width/height in
drm_atomic_helper_check_modeset(). Probably best to stuff that into
the mode_valid() function.

Cheers, Daniel
>
> Neil
>
>
> > -Daniel
> >
> >>
> >> Neil
> >>
> >>>
> >>> Bunch of igt to make sure we're not missing anything would be sweet on
> >>> top, e.g. e.g. trying to set a mode over the limit and making sure it
> >>> fails.
> >>>
> >>> Cheers, Daniel
> >>>
> >>>> ---
> >>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> >>>> index d344312..2e29968 100644
> >>>> --- a/drivers/gpu/drm/meson/meson_drv.c
> >>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
> >>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> >>>>              goto free_drm;
> >>>>
> >>>>      drm_mode_config_init(drm);
> >>>> -    drm->mode_config.max_width = 3840;
> >>>> -    drm->mode_config.max_height = 2160;
> >>>> +    drm->mode_config.max_width = 16384;
> >>>> +    drm->mode_config.max_height = 8192;
> >>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
> >>>>
> >>>>      /* Hardware Initialization */
> >>>> --
> >>>> 2.7.4
> >>>>
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel at lists.freedesktop.org
> >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 05/10/2018 09:58, Daniel Vetter wrote:
> On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>

[...]

>> OK, won't this be enough ?
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>>   * @min_height: minimum fb pixel height on this device
>>   * @max_width: maximum fb pixel width on this device
>>   * @max_height: maximum fb pixel height on this device
>> + * @max_fb_width: maximum fb buffer width if differs from max_width
>> + * @max_fb_height: maximum fb buffer height if differs from max_height
>>   * @funcs: core driver provided mode setting functions
>>   * @fb_base: base address of the framebuffer
>>   * @poll_enabled: track polling support for this device
>> @@ -508,6 +510,7 @@ struct drm_mode_config {
>>
>>         int min_width, min_height;
>>         int max_width, max_height;
>> +       int max_fb_width, max_fb_height;
>>         const struct drm_mode_config_funcs *funcs;
>>         resource_size_t fb_base;
>>
>> --- a/drivers/gpu/drm/drm_framebuffer.c
>> +++ b/drivers/gpu/drm/drm_framebuffer.c
>> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
>>                 return ERR_PTR(-EINVAL);
>>         }
>>
>> -       if ((config->min_width > r->width) || (r->width > config->max_width)) {
>> +       if ((config->min_width > r->width) ||
>> +           (!config->max_fb_width && r->width > config->max_width) ||
>> +           (config->max_fb_width && r->width > config->max_fb_width)) {
>>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
>> -                         r->width, config->min_width, config->max_width);
>> +                         r->width, config->min_width, config->max_fb_width ?
>> +                         config->max_fb_width : config->max_width);
>>                 return ERR_PTR(-EINVAL);
>>         }
>> -       if ((config->min_height > r->height) || (r->height > config->max_height)) {
>> +       if ((config->min_height > r->height) ||
>> +           (!config->max_fb_height && r->height > config->max_height) ||
>> +           (config->max_fb_height && r->height > config->max_fb_height)) {
>>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
>> -                         r->height, config->min_height, config->max_height);
>> +                         r->height, config->min_height, config->max_fb_height ?
>> +                         config->max_fb_height : config->max_height);
>>                 return ERR_PTR(-EINVAL);
>>         }
>>
>> and in the driver :
>>
>> +       drm->mode_config.max_width = 4096;
>> +       drm->mode_config.max_height = 3840;
>> +       drm->mode_config.max_fb_width = 16384;
>> +       drm->mode_config.max_fb_height = 8192;
>>
>> With this I leave the mode filtering intact.
> 
> Not enough. See
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
> and scroll down to mode_valid. You need to filter modes both in the
> detect paths, and the atomic_check paths.
> 
> Detect is explicitly filtered out, but atomic_check was only
> implicitly filtered, through the max fb size checks. Ok, you could
> light up a mode that's bigger than max fb, but in practice, no
> userspace ever did that. But with your code we're missing crucial
> validation now, and userspace could fall over that. What I think we
> need is to add mode filter against mode_config.max_width/height in
> drm_atomic_helper_check_modeset(). Probably best to stuff that into
> the mode_valid() function.

Ok I understood now, thanks for pointer, I'll try to add this.

Neil

> 
> Cheers, Daniel
>>
>> Neil
>>
>>
>>> -Daniel
>>>
>>>>
>>>> Neil
>>>>
>>>>>
>>>>> Bunch of igt to make sure we're not missing anything would be sweet on
>>>>> top, e.g. e.g. trying to set a mode over the limit and making sure it
>>>>> fails.
>>>>>
>>>>> Cheers, Daniel
>>>>>
>>>>>> ---
>>>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>>>>>> index d344312..2e29968 100644
>>>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>>>>>              goto free_drm;
>>>>>>
>>>>>>      drm_mode_config_init(drm);
>>>>>> -    drm->mode_config.max_width = 3840;
>>>>>> -    drm->mode_config.max_height = 2160;
>>>>>> +    drm->mode_config.max_width = 16384;
>>>>>> +    drm->mode_config.max_height = 8192;
>>>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>>>
>>>>>>      /* Hardware Initialization */
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel at lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>>
>>
> 
> 

Re: [PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 05/10/2018 09:58, Daniel Vetter wrote:
> On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>

[...]

>> OK, won't this be enough ?
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>>   * @min_height: minimum fb pixel height on this device
>>   * @max_width: maximum fb pixel width on this device
>>   * @max_height: maximum fb pixel height on this device
>> + * @max_fb_width: maximum fb buffer width if differs from max_width
>> + * @max_fb_height: maximum fb buffer height if differs from max_height
>>   * @funcs: core driver provided mode setting functions
>>   * @fb_base: base address of the framebuffer
>>   * @poll_enabled: track polling support for this device
>> @@ -508,6 +510,7 @@ struct drm_mode_config {
>>
>>         int min_width, min_height;
>>         int max_width, max_height;
>> +       int max_fb_width, max_fb_height;
>>         const struct drm_mode_config_funcs *funcs;
>>         resource_size_t fb_base;
>>
>> --- a/drivers/gpu/drm/drm_framebuffer.c
>> +++ b/drivers/gpu/drm/drm_framebuffer.c
>> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
>>                 return ERR_PTR(-EINVAL);
>>         }
>>
>> -       if ((config->min_width > r->width) || (r->width > config->max_width)) {
>> +       if ((config->min_width > r->width) ||
>> +           (!config->max_fb_width && r->width > config->max_width) ||
>> +           (config->max_fb_width && r->width > config->max_fb_width)) {
>>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
>> -                         r->width, config->min_width, config->max_width);
>> +                         r->width, config->min_width, config->max_fb_width ?
>> +                         config->max_fb_width : config->max_width);
>>                 return ERR_PTR(-EINVAL);
>>         }
>> -       if ((config->min_height > r->height) || (r->height > config->max_height)) {
>> +       if ((config->min_height > r->height) ||
>> +           (!config->max_fb_height && r->height > config->max_height) ||
>> +           (config->max_fb_height && r->height > config->max_fb_height)) {
>>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
>> -                         r->height, config->min_height, config->max_height);
>> +                         r->height, config->min_height, config->max_fb_height ?
>> +                         config->max_fb_height : config->max_height);
>>                 return ERR_PTR(-EINVAL);
>>         }
>>
>> and in the driver :
>>
>> +       drm->mode_config.max_width = 4096;
>> +       drm->mode_config.max_height = 3840;
>> +       drm->mode_config.max_fb_width = 16384;
>> +       drm->mode_config.max_fb_height = 8192;
>>
>> With this I leave the mode filtering intact.
> 
> Not enough. See
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
> and scroll down to mode_valid. You need to filter modes both in the
> detect paths, and the atomic_check paths.
> 
> Detect is explicitly filtered out, but atomic_check was only
> implicitly filtered, through the max fb size checks. Ok, you could
> light up a mode that's bigger than max fb, but in practice, no
> userspace ever did that. But with your code we're missing crucial
> validation now, and userspace could fall over that. What I think we
> need is to add mode filter against mode_config.max_width/height in
> drm_atomic_helper_check_modeset(). Probably best to stuff that into
> the mode_valid() function.

Ok I understood now, thanks for pointer, I'll try to add this.

Neil

> 
> Cheers, Daniel
>>
>> Neil
>>
>>
>>> -Daniel
>>>
>>>>
>>>> Neil
>>>>
>>>>>
>>>>> Bunch of igt to make sure we're not missing anything would be sweet on
>>>>> top, e.g. e.g. trying to set a mode over the limit and making sure it
>>>>> fails.
>>>>>
>>>>> Cheers, Daniel
>>>>>
>>>>>> ---
>>>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>>>>>> index d344312..2e29968 100644
>>>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>>>>>              goto free_drm;
>>>>>>
>>>>>>      drm_mode_config_init(drm);
>>>>>> -    drm->mode_config.max_width = 3840;
>>>>>> -    drm->mode_config.max_height = 2160;
>>>>>> +    drm->mode_config.max_width = 16384;
>>>>>> +    drm->mode_config.max_height = 8192;
>>>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>>>
>>>>>>      /* Hardware Initialization */
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>>
>>
> 
> 

Re: [PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 05/10/2018 09:58, Daniel Vetter wrote:
> On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>

[...]

>> OK, won't this be enough ?
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>>   * @min_height: minimum fb pixel height on this device
>>   * @max_width: maximum fb pixel width on this device
>>   * @max_height: maximum fb pixel height on this device
>> + * @max_fb_width: maximum fb buffer width if differs from max_width
>> + * @max_fb_height: maximum fb buffer height if differs from max_height
>>   * @funcs: core driver provided mode setting functions
>>   * @fb_base: base address of the framebuffer
>>   * @poll_enabled: track polling support for this device
>> @@ -508,6 +510,7 @@ struct drm_mode_config {
>>
>>         int min_width, min_height;
>>         int max_width, max_height;
>> +       int max_fb_width, max_fb_height;
>>         const struct drm_mode_config_funcs *funcs;
>>         resource_size_t fb_base;
>>
>> --- a/drivers/gpu/drm/drm_framebuffer.c
>> +++ b/drivers/gpu/drm/drm_framebuffer.c
>> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
>>                 return ERR_PTR(-EINVAL);
>>         }
>>
>> -       if ((config->min_width > r->width) || (r->width > config->max_width)) {
>> +       if ((config->min_width > r->width) ||
>> +           (!config->max_fb_width && r->width > config->max_width) ||
>> +           (config->max_fb_width && r->width > config->max_fb_width)) {
>>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
>> -                         r->width, config->min_width, config->max_width);
>> +                         r->width, config->min_width, config->max_fb_width ?
>> +                         config->max_fb_width : config->max_width);
>>                 return ERR_PTR(-EINVAL);
>>         }
>> -       if ((config->min_height > r->height) || (r->height > config->max_height)) {
>> +       if ((config->min_height > r->height) ||
>> +           (!config->max_fb_height && r->height > config->max_height) ||
>> +           (config->max_fb_height && r->height > config->max_fb_height)) {
>>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
>> -                         r->height, config->min_height, config->max_height);
>> +                         r->height, config->min_height, config->max_fb_height ?
>> +                         config->max_fb_height : config->max_height);
>>                 return ERR_PTR(-EINVAL);
>>         }
>>
>> and in the driver :
>>
>> +       drm->mode_config.max_width = 4096;
>> +       drm->mode_config.max_height = 3840;
>> +       drm->mode_config.max_fb_width = 16384;
>> +       drm->mode_config.max_fb_height = 8192;
>>
>> With this I leave the mode filtering intact.
> 
> Not enough. See
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
> and scroll down to mode_valid. You need to filter modes both in the
> detect paths, and the atomic_check paths.
> 
> Detect is explicitly filtered out, but atomic_check was only
> implicitly filtered, through the max fb size checks. Ok, you could
> light up a mode that's bigger than max fb, but in practice, no
> userspace ever did that. But with your code we're missing crucial
> validation now, and userspace could fall over that. What I think we
> need is to add mode filter against mode_config.max_width/height in
> drm_atomic_helper_check_modeset(). Probably best to stuff that into
> the mode_valid() function.

Ok I understood now, thanks for pointer, I'll try to add this.

Neil

> 
> Cheers, Daniel
>>
>> Neil
>>
>>
>>> -Daniel
>>>
>>>>
>>>> Neil
>>>>
>>>>>
>>>>> Bunch of igt to make sure we're not missing anything would be sweet on
>>>>> top, e.g. e.g. trying to set a mode over the limit and making sure it
>>>>> fails.
>>>>>
>>>>> Cheers, Daniel
>>>>>
>>>>>> ---
>>>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>>>>>> index d344312..2e29968 100644
>>>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>>>>>              goto free_drm;
>>>>>>
>>>>>>      drm_mode_config_init(drm);
>>>>>> -    drm->mode_config.max_width = 3840;
>>>>>> -    drm->mode_config.max_height = 2160;
>>>>>> +    drm->mode_config.max_width = 16384;
>>>>>> +    drm->mode_config.max_height = 8192;
>>>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>>>
>>>>>>      /* Hardware Initialization */
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>>
>>
> 
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/meson: fix max mode_config height/width

[Neil Armstrong]

On 05/10/2018 09:58, Daniel Vetter wrote:
> On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>

[...]

>> OK, won't this be enough ?
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>>   * @min_height: minimum fb pixel height on this device
>>   * @max_width: maximum fb pixel width on this device
>>   * @max_height: maximum fb pixel height on this device
>> + * @max_fb_width: maximum fb buffer width if differs from max_width
>> + * @max_fb_height: maximum fb buffer height if differs from max_height
>>   * @funcs: core driver provided mode setting functions
>>   * @fb_base: base address of the framebuffer
>>   * @poll_enabled: track polling support for this device
>> @@ -508,6 +510,7 @@ struct drm_mode_config {
>>
>>         int min_width, min_height;
>>         int max_width, max_height;
>> +       int max_fb_width, max_fb_height;
>>         const struct drm_mode_config_funcs *funcs;
>>         resource_size_t fb_base;
>>
>> --- a/drivers/gpu/drm/drm_framebuffer.c
>> +++ b/drivers/gpu/drm/drm_framebuffer.c
>> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct drm_device *dev,
>>                 return ERR_PTR(-EINVAL);
>>         }
>>
>> -       if ((config->min_width > r->width) || (r->width > config->max_width)) {
>> +       if ((config->min_width > r->width) ||
>> +           (!config->max_fb_width && r->width > config->max_width) ||
>> +           (config->max_fb_width && r->width > config->max_fb_width)) {
>>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n",
>> -                         r->width, config->min_width, config->max_width);
>> +                         r->width, config->min_width, config->max_fb_width ?
>> +                         config->max_fb_width : config->max_width);
>>                 return ERR_PTR(-EINVAL);
>>         }
>> -       if ((config->min_height > r->height) || (r->height > config->max_height)) {
>> +       if ((config->min_height > r->height) ||
>> +           (!config->max_fb_height && r->height > config->max_height) ||
>> +           (config->max_fb_height && r->height > config->max_fb_height)) {
>>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n",
>> -                         r->height, config->min_height, config->max_height);
>> +                         r->height, config->min_height, config->max_fb_height ?
>> +                         config->max_fb_height : config->max_height);
>>                 return ERR_PTR(-EINVAL);
>>         }
>>
>> and in the driver :
>>
>> +       drm->mode_config.max_width = 4096;
>> +       drm->mode_config.max_height = 3840;
>> +       drm->mode_config.max_fb_width = 16384;
>> +       drm->mode_config.max_fb_height = 8192;
>>
>> With this I leave the mode filtering intact.
> 
> Not enough. See
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
> and scroll down to mode_valid. You need to filter modes both in the
> detect paths, and the atomic_check paths.
> 
> Detect is explicitly filtered out, but atomic_check was only
> implicitly filtered, through the max fb size checks. Ok, you could
> light up a mode that's bigger than max fb, but in practice, no
> userspace ever did that. But with your code we're missing crucial
> validation now, and userspace could fall over that. What I think we
> need is to add mode filter against mode_config.max_width/height in
> drm_atomic_helper_check_modeset(). Probably best to stuff that into
> the mode_valid() function.

Ok I understood now, thanks for pointer, I'll try to add this.

Neil

> 
> Cheers, Daniel
>>
>> Neil
>>
>>
>>> -Daniel
>>>
>>>>
>>>> Neil
>>>>
>>>>>
>>>>> Bunch of igt to make sure we're not missing anything would be sweet on
>>>>> top, e.g. e.g. trying to set a mode over the limit and making sure it
>>>>> fails.
>>>>>
>>>>> Cheers, Daniel
>>>>>
>>>>>> ---
>>>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>>>>>> index d344312..2e29968 100644
>>>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>>>>>              goto free_drm;
>>>>>>
>>>>>>      drm_mode_config_init(drm);
>>>>>> -    drm->mode_config.max_width = 3840;
>>>>>> -    drm->mode_config.max_height = 2160;
>>>>>> +    drm->mode_config.max_width = 16384;
>>>>>> +    drm->mode_config.max_height = 8192;
>>>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>>>
>>>>>>      /* Hardware Initialization */
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel at lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>>
>>
> 
> 

Re: [PATCH] drm/meson: fix max mode_config height/width

[Jeykumar Sankaran]

Reviving this thread from the context of the below conversion:

https://lore.kernel.org/linux-arm-msm/db26145b-3f64-a334-f698-76f972332881@baylibre.com/T/#u

On 2018-10-05 01:19, Neil Armstrong wrote:
> On 05/10/2018 09:58, Daniel Vetter wrote:
>> On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong 
>> <narmstrong@baylibre.com> wrote:
>>> 
> 
> [...]
> 
>>> OK, won't this be enough ?
>>> --- a/include/drm/drm_mode_config.h
>>> +++ b/include/drm/drm_mode_config.h
>>> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>>>   * @min_height: minimum fb pixel height on this device
>>>   * @max_width: maximum fb pixel width on this device
>>>   * @max_height: maximum fb pixel height on this device
>>> + * @max_fb_width: maximum fb buffer width if differs from max_width
>>> + * @max_fb_height: maximum fb buffer height if differs from 
>>> max_height
>>>   * @funcs: core driver provided mode setting functions
>>>   * @fb_base: base address of the framebuffer
>>>   * @poll_enabled: track polling support for this device
>>> @@ -508,6 +510,7 @@ struct drm_mode_config {
>>> 
>>>         int min_width, min_height;
>>>         int max_width, max_height;
>>> +       int max_fb_width, max_fb_height;
>>>         const struct drm_mode_config_funcs *funcs;
>>>         resource_size_t fb_base;
>>> 
>>> --- a/drivers/gpu/drm/drm_framebuffer.c
>>> +++ b/drivers/gpu/drm/drm_framebuffer.c
>>> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct 
>>> drm_device *dev,
>>>                 return ERR_PTR(-EINVAL);
>>>         }
>>> 
>>> -       if ((config->min_width > r->width) || (r->width > 
>>> config->max_width)) {
>>> +       if ((config->min_width > r->width) ||
>>> +           (!config->max_fb_width && r->width > config->max_width) 
>>> ||
>>> +           (config->max_fb_width && r->width > 
>>> config->max_fb_width)) {
>>>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= 
>>> %d && <= %d\n",
>>> -                         r->width, config->min_width, 
>>> config->max_width);
>>> +                         r->width, config->min_width, 
>>> config->max_fb_width ?
>>> +                         config->max_fb_width : config->max_width);
>>>                 return ERR_PTR(-EINVAL);
>>>         }
>>> -       if ((config->min_height > r->height) || (r->height > 
>>> config->max_height)) {
>>> +       if ((config->min_height > r->height) ||
>>> +           (!config->max_fb_height && r->height > 
>>> config->max_height) ||
>>> +           (config->max_fb_height && r->height > 
>>> config->max_fb_height)) {
>>>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be 
>>> >= %d && <= %d\n",
>>> -                         r->height, config->min_height, 
>>> config->max_height);
>>> +                         r->height, config->min_height, 
>>> config->max_fb_height ?
>>> +                         config->max_fb_height : 
>>> config->max_height);
>>>                 return ERR_PTR(-EINVAL);
>>>         }
>>> 
>>> and in the driver :
>>> 
>>> +       drm->mode_config.max_width = 4096;
>>> +       drm->mode_config.max_height = 3840;
>>> +       drm->mode_config.max_fb_width = 16384;
>>> +       drm->mode_config.max_fb_height = 8192;
>>> 
>>> With this I leave the mode filtering intact.
>> 
>> Not enough. See
>> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
>> and scroll down to mode_valid. You need to filter modes both in the
>> detect paths, and the atomic_check paths.
>> 
>> Detect is explicitly filtered out, but atomic_check was only
>> implicitly filtered, through the max fb size checks. Ok, you could
>> light up a mode that's bigger than max fb, but in practice, no
>> userspace ever did that.

Daniel, MSM and few other vendor hardware have upscale blocks where the 
driver can expose fb sizes smaller than
the mode resolution and use h/w upscaling to fill the screen. This would 
optimize the fetch bandwidth.

But with your code we're missing crucial
>> validation now, and userspace could fall over that. What I think we
>> need is to add mode filter against mode_config.max_width/height in
>> drm_atomic_helper_check_modeset(). Probably best to stuff that into
>> the mode_valid() function.
> 
Agreed! Since the above patch from Niel is taking care of cases where 
max/min fb values
are not set, by checking against the original max/min values, can we 
separate out this
core change from the driver level mode_valid changes? If Niel couldn't 
find the time, I can
repost the above change.

Thanks and Regards,
Jeykumar S.

> Ok I understood now, thanks for pointer, I'll try to add this.
> 
> Neil
> 
>> 
>> Cheers, Daniel
>>> 
>>> Neil
>>> 
>>> 
>>>> -Daniel
>>>> 
>>>>> 
>>>>> Neil
>>>>> 
>>>>>> 
>>>>>> Bunch of igt to make sure we're not missing anything would be 
>>>>>> sweet on
>>>>>> top, e.g. e.g. trying to set a mode over the limit and making sure 
>>>>>> it
>>>>>> fails.
>>>>>> 
>>>>>> Cheers, Daniel
>>>>>> 
>>>>>>> ---
>>>>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c 
>>>>>>> b/drivers/gpu/drm/meson/meson_drv.c
>>>>>>> index d344312..2e29968 100644
>>>>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct 
>>>>>>> device *dev, bool has_components)
>>>>>>>              goto free_drm;
>>>>>>> 
>>>>>>>      drm_mode_config_init(drm);
>>>>>>> -    drm->mode_config.max_width = 3840;
>>>>>>> -    drm->mode_config.max_height = 2160;
>>>>>>> +    drm->mode_config.max_width = 16384;
>>>>>>> +    drm->mode_config.max_height = 8192;
>>>>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>>>> 
>>>>>>>      /* Hardware Initialization */
>>>>>>> --
>>>>>>> 2.7.4
>>>>>>> 
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>> 
>>>> 
>>>> 
>>> 
>> 
>> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jeykumar S

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] drm/meson: fix max mode_config height/width

[Jeykumar Sankaran]

Reviving this thread from the context of the below conversion:

https://lore.kernel.org/linux-arm-msm/db26145b-3f64-a334-f698-76f972332881@baylibre.com/T/#u

On 2018-10-05 01:19, Neil Armstrong wrote:
> On 05/10/2018 09:58, Daniel Vetter wrote:
>> On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong 
>> <narmstrong@baylibre.com> wrote:
>>> 
> 
> [...]
> 
>>> OK, won't this be enough ?
>>> --- a/include/drm/drm_mode_config.h
>>> +++ b/include/drm/drm_mode_config.h
>>> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>>>   * @min_height: minimum fb pixel height on this device
>>>   * @max_width: maximum fb pixel width on this device
>>>   * @max_height: maximum fb pixel height on this device
>>> + * @max_fb_width: maximum fb buffer width if differs from max_width
>>> + * @max_fb_height: maximum fb buffer height if differs from 
>>> max_height
>>>   * @funcs: core driver provided mode setting functions
>>>   * @fb_base: base address of the framebuffer
>>>   * @poll_enabled: track polling support for this device
>>> @@ -508,6 +510,7 @@ struct drm_mode_config {
>>> 
>>>         int min_width, min_height;
>>>         int max_width, max_height;
>>> +       int max_fb_width, max_fb_height;
>>>         const struct drm_mode_config_funcs *funcs;
>>>         resource_size_t fb_base;
>>> 
>>> --- a/drivers/gpu/drm/drm_framebuffer.c
>>> +++ b/drivers/gpu/drm/drm_framebuffer.c
>>> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct 
>>> drm_device *dev,
>>>                 return ERR_PTR(-EINVAL);
>>>         }
>>> 
>>> -       if ((config->min_width > r->width) || (r->width > 
>>> config->max_width)) {
>>> +       if ((config->min_width > r->width) ||
>>> +           (!config->max_fb_width && r->width > config->max_width) 
>>> ||
>>> +           (config->max_fb_width && r->width > 
>>> config->max_fb_width)) {
>>>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= 
>>> %d && <= %d\n",
>>> -                         r->width, config->min_width, 
>>> config->max_width);
>>> +                         r->width, config->min_width, 
>>> config->max_fb_width ?
>>> +                         config->max_fb_width : config->max_width);
>>>                 return ERR_PTR(-EINVAL);
>>>         }
>>> -       if ((config->min_height > r->height) || (r->height > 
>>> config->max_height)) {
>>> +       if ((config->min_height > r->height) ||
>>> +           (!config->max_fb_height && r->height > 
>>> config->max_height) ||
>>> +           (config->max_fb_height && r->height > 
>>> config->max_fb_height)) {
>>>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be 
>>> >= %d && <= %d\n",
>>> -                         r->height, config->min_height, 
>>> config->max_height);
>>> +                         r->height, config->min_height, 
>>> config->max_fb_height ?
>>> +                         config->max_fb_height : 
>>> config->max_height);
>>>                 return ERR_PTR(-EINVAL);
>>>         }
>>> 
>>> and in the driver :
>>> 
>>> +       drm->mode_config.max_width = 4096;
>>> +       drm->mode_config.max_height = 3840;
>>> +       drm->mode_config.max_fb_width = 16384;
>>> +       drm->mode_config.max_fb_height = 8192;
>>> 
>>> With this I leave the mode filtering intact.
>> 
>> Not enough. See
>> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
>> and scroll down to mode_valid. You need to filter modes both in the
>> detect paths, and the atomic_check paths.
>> 
>> Detect is explicitly filtered out, but atomic_check was only
>> implicitly filtered, through the max fb size checks. Ok, you could
>> light up a mode that's bigger than max fb, but in practice, no
>> userspace ever did that.

Daniel, MSM and few other vendor hardware have upscale blocks where the 
driver can expose fb sizes smaller than
the mode resolution and use h/w upscaling to fill the screen. This would 
optimize the fetch bandwidth.

But with your code we're missing crucial
>> validation now, and userspace could fall over that. What I think we
>> need is to add mode filter against mode_config.max_width/height in
>> drm_atomic_helper_check_modeset(). Probably best to stuff that into
>> the mode_valid() function.
> 
Agreed! Since the above patch from Niel is taking care of cases where 
max/min fb values
are not set, by checking against the original max/min values, can we 
separate out this
core change from the driver level mode_valid changes? If Niel couldn't 
find the time, I can
repost the above change.

Thanks and Regards,
Jeykumar S.

> Ok I understood now, thanks for pointer, I'll try to add this.
> 
> Neil
> 
>> 
>> Cheers, Daniel
>>> 
>>> Neil
>>> 
>>> 
>>>> -Daniel
>>>> 
>>>>> 
>>>>> Neil
>>>>> 
>>>>>> 
>>>>>> Bunch of igt to make sure we're not missing anything would be 
>>>>>> sweet on
>>>>>> top, e.g. e.g. trying to set a mode over the limit and making sure 
>>>>>> it
>>>>>> fails.
>>>>>> 
>>>>>> Cheers, Daniel
>>>>>> 
>>>>>>> ---
>>>>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c 
>>>>>>> b/drivers/gpu/drm/meson/meson_drv.c
>>>>>>> index d344312..2e29968 100644
>>>>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct 
>>>>>>> device *dev, bool has_components)
>>>>>>>              goto free_drm;
>>>>>>> 
>>>>>>>      drm_mode_config_init(drm);
>>>>>>> -    drm->mode_config.max_width = 3840;
>>>>>>> -    drm->mode_config.max_height = 2160;
>>>>>>> +    drm->mode_config.max_width = 16384;
>>>>>>> +    drm->mode_config.max_height = 8192;
>>>>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>>>> 
>>>>>>>      /* Hardware Initialization */
>>>>>>> --
>>>>>>> 2.7.4
>>>>>>> 
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>> 
>>>> 
>>>> 
>>> 
>> 
>> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jeykumar S

Re: [PATCH] drm/meson: fix max mode_config height/width

[Jeykumar Sankaran]

Reviving this thread from the context of the below conversion:

https://lore.kernel.org/linux-arm-msm/db26145b-3f64-a334-f698-76f972332881@baylibre.com/T/#u

On 2018-10-05 01:19, Neil Armstrong wrote:
> On 05/10/2018 09:58, Daniel Vetter wrote:
>> On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong 
>> <narmstrong@baylibre.com> wrote:
>>> 
> 
> [...]
> 
>>> OK, won't this be enough ?
>>> --- a/include/drm/drm_mode_config.h
>>> +++ b/include/drm/drm_mode_config.h
>>> @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>>>   * @min_height: minimum fb pixel height on this device
>>>   * @max_width: maximum fb pixel width on this device
>>>   * @max_height: maximum fb pixel height on this device
>>> + * @max_fb_width: maximum fb buffer width if differs from max_width
>>> + * @max_fb_height: maximum fb buffer height if differs from 
>>> max_height
>>>   * @funcs: core driver provided mode setting functions
>>>   * @fb_base: base address of the framebuffer
>>>   * @poll_enabled: track polling support for this device
>>> @@ -508,6 +510,7 @@ struct drm_mode_config {
>>> 
>>>         int min_width, min_height;
>>>         int max_width, max_height;
>>> +       int max_fb_width, max_fb_height;
>>>         const struct drm_mode_config_funcs *funcs;
>>>         resource_size_t fb_base;
>>> 
>>> --- a/drivers/gpu/drm/drm_framebuffer.c
>>> +++ b/drivers/gpu/drm/drm_framebuffer.c
>>> @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct 
>>> drm_device *dev,
>>>                 return ERR_PTR(-EINVAL);
>>>         }
>>> 
>>> -       if ((config->min_width > r->width) || (r->width > 
>>> config->max_width)) {
>>> +       if ((config->min_width > r->width) ||
>>> +           (!config->max_fb_width && r->width > config->max_width) 
>>> ||
>>> +           (config->max_fb_width && r->width > 
>>> config->max_fb_width)) {
>>>                 DRM_DEBUG_KMS("bad framebuffer width %d, should be >= 
>>> %d && <= %d\n",
>>> -                         r->width, config->min_width, 
>>> config->max_width);
>>> +                         r->width, config->min_width, 
>>> config->max_fb_width ?
>>> +                         config->max_fb_width : config->max_width);
>>>                 return ERR_PTR(-EINVAL);
>>>         }
>>> -       if ((config->min_height > r->height) || (r->height > 
>>> config->max_height)) {
>>> +       if ((config->min_height > r->height) ||
>>> +           (!config->max_fb_height && r->height > 
>>> config->max_height) ||
>>> +           (config->max_fb_height && r->height > 
>>> config->max_fb_height)) {
>>>                 DRM_DEBUG_KMS("bad framebuffer height %d, should be 
>>> >= %d && <= %d\n",
>>> -                         r->height, config->min_height, 
>>> config->max_height);
>>> +                         r->height, config->min_height, 
>>> config->max_fb_height ?
>>> +                         config->max_fb_height : 
>>> config->max_height);
>>>                 return ERR_PTR(-EINVAL);
>>>         }
>>> 
>>> and in the driver :
>>> 
>>> +       drm->mode_config.max_width = 4096;
>>> +       drm->mode_config.max_height = 3840;
>>> +       drm->mode_config.max_fb_width = 16384;
>>> +       drm->mode_config.max_fb_height = 8192;
>>> 
>>> With this I leave the mode filtering intact.
>> 
>> Not enough. See
>> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
>> and scroll down to mode_valid. You need to filter modes both in the
>> detect paths, and the atomic_check paths.
>> 
>> Detect is explicitly filtered out, but atomic_check was only
>> implicitly filtered, through the max fb size checks. Ok, you could
>> light up a mode that's bigger than max fb, but in practice, no
>> userspace ever did that.

Daniel, MSM and few other vendor hardware have upscale blocks where the 
driver can expose fb sizes smaller than
the mode resolution and use h/w upscaling to fill the screen. This would 
optimize the fetch bandwidth.

But with your code we're missing crucial
>> validation now, and userspace could fall over that. What I think we
>> need is to add mode filter against mode_config.max_width/height in
>> drm_atomic_helper_check_modeset(). Probably best to stuff that into
>> the mode_valid() function.
> 
Agreed! Since the above patch from Niel is taking care of cases where 
max/min fb values
are not set, by checking against the original max/min values, can we 
separate out this
core change from the driver level mode_valid changes? If Niel couldn't 
find the time, I can
repost the above change.

Thanks and Regards,
Jeykumar S.

> Ok I understood now, thanks for pointer, I'll try to add this.
> 
> Neil
> 
>> 
>> Cheers, Daniel
>>> 
>>> Neil
>>> 
>>> 
>>>> -Daniel
>>>> 
>>>>> 
>>>>> Neil
>>>>> 
>>>>>> 
>>>>>> Bunch of igt to make sure we're not missing anything would be 
>>>>>> sweet on
>>>>>> top, e.g. e.g. trying to set a mode over the limit and making sure 
>>>>>> it
>>>>>> fails.
>>>>>> 
>>>>>> Cheers, Daniel
>>>>>> 
>>>>>>> ---
>>>>>>>  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c 
>>>>>>> b/drivers/gpu/drm/meson/meson_drv.c
>>>>>>> index d344312..2e29968 100644
>>>>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>>>>> @@ -243,8 +243,8 @@ static int meson_drv_bind_master(struct 
>>>>>>> device *dev, bool has_components)
>>>>>>>              goto free_drm;
>>>>>>> 
>>>>>>>      drm_mode_config_init(drm);
>>>>>>> -    drm->mode_config.max_width = 3840;
>>>>>>> -    drm->mode_config.max_height = 2160;
>>>>>>> +    drm->mode_config.max_width = 16384;
>>>>>>> +    drm->mode_config.max_height = 8192;
>>>>>>>      drm->mode_config.funcs = &meson_mode_config_funcs;
>>>>>>> 
>>>>>>>      /* Hardware Initialization */
>>>>>>> --
>>>>>>> 2.7.4
>>>>>>> 
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>> 
>>>> 
>>>> 
>>> 
>> 
>> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jeykumar S

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Tue, Sep 24, 2019 at 10:28:48AM -0700, Jeykumar Sankaran wrote:
> Reviving this thread from the context of the below conversion:
> 
> https://lore.kernel.org/linux-arm-msm/db26145b-3f64-a334-f698-76f972332881@baylibre.com/T/#u
> 
> On 2018-10-05 01:19, Neil Armstrong wrote:
> > On 05/10/2018 09:58, Daniel Vetter wrote:
> > > On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong
> > > <narmstrong@baylibre.com> wrote:
> > > > 
> > 
> > [...]
> > 
> > > > OK, won't this be enough ?
> > > > --- a/include/drm/drm_mode_config.h
> > > > +++ b/include/drm/drm_mode_config.h
> > > > @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
> > > >   * @min_height: minimum fb pixel height on this device
> > > >   * @max_width: maximum fb pixel width on this device
> > > >   * @max_height: maximum fb pixel height on this device
> > > > + * @max_fb_width: maximum fb buffer width if differs from max_width
> > > > + * @max_fb_height: maximum fb buffer height if differs from
> > > > max_height
> > > >   * @funcs: core driver provided mode setting functions
> > > >   * @fb_base: base address of the framebuffer
> > > >   * @poll_enabled: track polling support for this device
> > > > @@ -508,6 +510,7 @@ struct drm_mode_config {
> > > > 
> > > >         int min_width, min_height;
> > > >         int max_width, max_height;
> > > > +       int max_fb_width, max_fb_height;
> > > >         const struct drm_mode_config_funcs *funcs;
> > > >         resource_size_t fb_base;
> > > > 
> > > > --- a/drivers/gpu/drm/drm_framebuffer.c
> > > > +++ b/drivers/gpu/drm/drm_framebuffer.c
> > > > @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct
> > > > drm_device *dev,
> > > >                 return ERR_PTR(-EINVAL);
> > > >         }
> > > > 
> > > > -       if ((config->min_width > r->width) || (r->width >
> > > > config->max_width)) {
> > > > +       if ((config->min_width > r->width) ||
> > > > +           (!config->max_fb_width && r->width >
> > > > config->max_width) ||
> > > > +           (config->max_fb_width && r->width >
> > > > config->max_fb_width)) {
> > > >                 DRM_DEBUG_KMS("bad framebuffer width %d, should
> > > > be >= %d && <= %d\n",
> > > > -                         r->width, config->min_width,
> > > > config->max_width);
> > > > +                         r->width, config->min_width,
> > > > config->max_fb_width ?
> > > > +                         config->max_fb_width : config->max_width);
> > > >                 return ERR_PTR(-EINVAL);
> > > >         }
> > > > -       if ((config->min_height > r->height) || (r->height >
> > > > config->max_height)) {
> > > > +       if ((config->min_height > r->height) ||
> > > > +           (!config->max_fb_height && r->height >
> > > > config->max_height) ||
> > > > +           (config->max_fb_height && r->height >
> > > > config->max_fb_height)) {
> > > >                 DRM_DEBUG_KMS("bad framebuffer height %d, should
> > > > be >= %d && <= %d\n",
> > > > -                         r->height, config->min_height,
> > > > config->max_height);
> > > > +                         r->height, config->min_height,
> > > > config->max_fb_height ?
> > > > +                         config->max_fb_height :
> > > > config->max_height);
> > > >                 return ERR_PTR(-EINVAL);
> > > >         }
> > > > 
> > > > and in the driver :
> > > > 
> > > > +       drm->mode_config.max_width = 4096;
> > > > +       drm->mode_config.max_height = 3840;
> > > > +       drm->mode_config.max_fb_width = 16384;
> > > > +       drm->mode_config.max_fb_height = 8192;
> > > > 
> > > > With this I leave the mode filtering intact.
> > > 
> > > Not enough. See
> > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
> > > and scroll down to mode_valid. You need to filter modes both in the
> > > detect paths, and the atomic_check paths.
> > > 
> > > Detect is explicitly filtered out, but atomic_check was only
> > > implicitly filtered, through the max fb size checks. Ok, you could
> > > light up a mode that's bigger than max fb, but in practice, no
> > > userspace ever did that.
> 
> Daniel, MSM and few other vendor hardware have upscale blocks where the
> driver can expose fb sizes smaller than
> the mode resolution and use h/w upscaling to fill the screen. This would
> optimize the fetch bandwidth.
> 
> But with your code we're missing crucial
> > > validation now, and userspace could fall over that. What I think we
> > > need is to add mode filter against mode_config.max_width/height in
> > > drm_atomic_helper_check_modeset(). Probably best to stuff that into
> > > the mode_valid() function.
> > 
> Agreed! Since the above patch from Niel is taking care of cases where
> max/min fb values
> are not set, by checking against the original max/min values, can we
> separate out this
> core change from the driver level mode_valid changes? If Niel couldn't find
> the time, I can
> repost the above change.

Sure, I think Neil wouldn't mind if you take this over and get it ready
for merging. Just need to make sure we're not leaving any validation gaps
in core/helper code.
-Daniel

> 
> Thanks and Regards,
> Jeykumar S.
> 
> > Ok I understood now, thanks for pointer, I'll try to add this.
> > 
> > Neil
> > 
> > > 
> > > Cheers, Daniel
> > > > 
> > > > Neil
> > > > 
> > > > 
> > > > > -Daniel
> > > > > 
> > > > > > 
> > > > > > Neil
> > > > > > 
> > > > > > > 
> > > > > > > Bunch of igt to make sure we're not missing anything
> > > > > > > would be sweet on
> > > > > > > top, e.g. e.g. trying to set a mode over the limit
> > > > > > > and making sure it
> > > > > > > fails.
> > > > > > > 
> > > > > > > Cheers, Daniel
> > > > > > > 
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> > > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > b/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > index d344312..2e29968 100644
> > > > > > > > --- a/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > +++ b/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > @@ -243,8 +243,8 @@ static int
> > > > > > > > meson_drv_bind_master(struct device *dev, bool
> > > > > > > > has_components)
> > > > > > > >              goto free_drm;
> > > > > > > > 
> > > > > > > >      drm_mode_config_init(drm);
> > > > > > > > -    drm->mode_config.max_width = 3840;
> > > > > > > > -    drm->mode_config.max_height = 2160;
> > > > > > > > +    drm->mode_config.max_width = 16384;
> > > > > > > > +    drm->mode_config.max_height = 8192;
> > > > > > > >      drm->mode_config.funcs = &meson_mode_config_funcs;
> > > > > > > > 
> > > > > > > >      /* Hardware Initialization */
> > > > > > > > --
> > > > > > > > 2.7.4
> > > > > > > > 
> > > > > > > > _______________________________________________
> > > > > > > > dri-devel mailing list
> > > > > > > > dri-devel@lists.freedesktop.org
> > > > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > > > > 
> > > > > > 
> > > > > > _______________________________________________
> > > > > > dri-devel mailing list
> > > > > > dri-devel@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > > 
> > > > > 
> > > > > 
> > > > 
> > > 
> > > 
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Jeykumar S

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Tue, Sep 24, 2019 at 10:28:48AM -0700, Jeykumar Sankaran wrote:
> Reviving this thread from the context of the below conversion:
> 
> https://lore.kernel.org/linux-arm-msm/db26145b-3f64-a334-f698-76f972332881@baylibre.com/T/#u
> 
> On 2018-10-05 01:19, Neil Armstrong wrote:
> > On 05/10/2018 09:58, Daniel Vetter wrote:
> > > On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong
> > > <narmstrong@baylibre.com> wrote:
> > > > 
> > 
> > [...]
> > 
> > > > OK, won't this be enough ?
> > > > --- a/include/drm/drm_mode_config.h
> > > > +++ b/include/drm/drm_mode_config.h
> > > > @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
> > > >   * @min_height: minimum fb pixel height on this device
> > > >   * @max_width: maximum fb pixel width on this device
> > > >   * @max_height: maximum fb pixel height on this device
> > > > + * @max_fb_width: maximum fb buffer width if differs from max_width
> > > > + * @max_fb_height: maximum fb buffer height if differs from
> > > > max_height
> > > >   * @funcs: core driver provided mode setting functions
> > > >   * @fb_base: base address of the framebuffer
> > > >   * @poll_enabled: track polling support for this device
> > > > @@ -508,6 +510,7 @@ struct drm_mode_config {
> > > > 
> > > >         int min_width, min_height;
> > > >         int max_width, max_height;
> > > > +       int max_fb_width, max_fb_height;
> > > >         const struct drm_mode_config_funcs *funcs;
> > > >         resource_size_t fb_base;
> > > > 
> > > > --- a/drivers/gpu/drm/drm_framebuffer.c
> > > > +++ b/drivers/gpu/drm/drm_framebuffer.c
> > > > @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct
> > > > drm_device *dev,
> > > >                 return ERR_PTR(-EINVAL);
> > > >         }
> > > > 
> > > > -       if ((config->min_width > r->width) || (r->width >
> > > > config->max_width)) {
> > > > +       if ((config->min_width > r->width) ||
> > > > +           (!config->max_fb_width && r->width >
> > > > config->max_width) ||
> > > > +           (config->max_fb_width && r->width >
> > > > config->max_fb_width)) {
> > > >                 DRM_DEBUG_KMS("bad framebuffer width %d, should
> > > > be >= %d && <= %d\n",
> > > > -                         r->width, config->min_width,
> > > > config->max_width);
> > > > +                         r->width, config->min_width,
> > > > config->max_fb_width ?
> > > > +                         config->max_fb_width : config->max_width);
> > > >                 return ERR_PTR(-EINVAL);
> > > >         }
> > > > -       if ((config->min_height > r->height) || (r->height >
> > > > config->max_height)) {
> > > > +       if ((config->min_height > r->height) ||
> > > > +           (!config->max_fb_height && r->height >
> > > > config->max_height) ||
> > > > +           (config->max_fb_height && r->height >
> > > > config->max_fb_height)) {
> > > >                 DRM_DEBUG_KMS("bad framebuffer height %d, should
> > > > be >= %d && <= %d\n",
> > > > -                         r->height, config->min_height,
> > > > config->max_height);
> > > > +                         r->height, config->min_height,
> > > > config->max_fb_height ?
> > > > +                         config->max_fb_height :
> > > > config->max_height);
> > > >                 return ERR_PTR(-EINVAL);
> > > >         }
> > > > 
> > > > and in the driver :
> > > > 
> > > > +       drm->mode_config.max_width = 4096;
> > > > +       drm->mode_config.max_height = 3840;
> > > > +       drm->mode_config.max_fb_width = 16384;
> > > > +       drm->mode_config.max_fb_height = 8192;
> > > > 
> > > > With this I leave the mode filtering intact.
> > > 
> > > Not enough. See
> > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
> > > and scroll down to mode_valid. You need to filter modes both in the
> > > detect paths, and the atomic_check paths.
> > > 
> > > Detect is explicitly filtered out, but atomic_check was only
> > > implicitly filtered, through the max fb size checks. Ok, you could
> > > light up a mode that's bigger than max fb, but in practice, no
> > > userspace ever did that.
> 
> Daniel, MSM and few other vendor hardware have upscale blocks where the
> driver can expose fb sizes smaller than
> the mode resolution and use h/w upscaling to fill the screen. This would
> optimize the fetch bandwidth.
> 
> But with your code we're missing crucial
> > > validation now, and userspace could fall over that. What I think we
> > > need is to add mode filter against mode_config.max_width/height in
> > > drm_atomic_helper_check_modeset(). Probably best to stuff that into
> > > the mode_valid() function.
> > 
> Agreed! Since the above patch from Niel is taking care of cases where
> max/min fb values
> are not set, by checking against the original max/min values, can we
> separate out this
> core change from the driver level mode_valid changes? If Niel couldn't find
> the time, I can
> repost the above change.

Sure, I think Neil wouldn't mind if you take this over and get it ready
for merging. Just need to make sure we're not leaving any validation gaps
in core/helper code.
-Daniel

> 
> Thanks and Regards,
> Jeykumar S.
> 
> > Ok I understood now, thanks for pointer, I'll try to add this.
> > 
> > Neil
> > 
> > > 
> > > Cheers, Daniel
> > > > 
> > > > Neil
> > > > 
> > > > 
> > > > > -Daniel
> > > > > 
> > > > > > 
> > > > > > Neil
> > > > > > 
> > > > > > > 
> > > > > > > Bunch of igt to make sure we're not missing anything
> > > > > > > would be sweet on
> > > > > > > top, e.g. e.g. trying to set a mode over the limit
> > > > > > > and making sure it
> > > > > > > fails.
> > > > > > > 
> > > > > > > Cheers, Daniel
> > > > > > > 
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> > > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > b/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > index d344312..2e29968 100644
> > > > > > > > --- a/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > +++ b/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > @@ -243,8 +243,8 @@ static int
> > > > > > > > meson_drv_bind_master(struct device *dev, bool
> > > > > > > > has_components)
> > > > > > > >              goto free_drm;
> > > > > > > > 
> > > > > > > >      drm_mode_config_init(drm);
> > > > > > > > -    drm->mode_config.max_width = 3840;
> > > > > > > > -    drm->mode_config.max_height = 2160;
> > > > > > > > +    drm->mode_config.max_width = 16384;
> > > > > > > > +    drm->mode_config.max_height = 8192;
> > > > > > > >      drm->mode_config.funcs = &meson_mode_config_funcs;
> > > > > > > > 
> > > > > > > >      /* Hardware Initialization */
> > > > > > > > --
> > > > > > > > 2.7.4
> > > > > > > > 
> > > > > > > > _______________________________________________
> > > > > > > > dri-devel mailing list
> > > > > > > > dri-devel@lists.freedesktop.org
> > > > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > > > > 
> > > > > > 
> > > > > > _______________________________________________
> > > > > > dri-devel mailing list
> > > > > > dri-devel@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > > 
> > > > > 
> > > > > 
> > > > 
> > > 
> > > 
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Jeykumar S

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/meson: fix max mode_config height/width

[Daniel Vetter]

On Tue, Sep 24, 2019 at 10:28:48AM -0700, Jeykumar Sankaran wrote:
> Reviving this thread from the context of the below conversion:
> 
> https://lore.kernel.org/linux-arm-msm/db26145b-3f64-a334-f698-76f972332881@baylibre.com/T/#u
> 
> On 2018-10-05 01:19, Neil Armstrong wrote:
> > On 05/10/2018 09:58, Daniel Vetter wrote:
> > > On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong
> > > <narmstrong@baylibre.com> wrote:
> > > > 
> > 
> > [...]
> > 
> > > > OK, won't this be enough ?
> > > > --- a/include/drm/drm_mode_config.h
> > > > +++ b/include/drm/drm_mode_config.h
> > > > @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
> > > >   * @min_height: minimum fb pixel height on this device
> > > >   * @max_width: maximum fb pixel width on this device
> > > >   * @max_height: maximum fb pixel height on this device
> > > > + * @max_fb_width: maximum fb buffer width if differs from max_width
> > > > + * @max_fb_height: maximum fb buffer height if differs from
> > > > max_height
> > > >   * @funcs: core driver provided mode setting functions
> > > >   * @fb_base: base address of the framebuffer
> > > >   * @poll_enabled: track polling support for this device
> > > > @@ -508,6 +510,7 @@ struct drm_mode_config {
> > > > 
> > > >         int min_width, min_height;
> > > >         int max_width, max_height;
> > > > +       int max_fb_width, max_fb_height;
> > > >         const struct drm_mode_config_funcs *funcs;
> > > >         resource_size_t fb_base;
> > > > 
> > > > --- a/drivers/gpu/drm/drm_framebuffer.c
> > > > +++ b/drivers/gpu/drm/drm_framebuffer.c
> > > > @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct
> > > > drm_device *dev,
> > > >                 return ERR_PTR(-EINVAL);
> > > >         }
> > > > 
> > > > -       if ((config->min_width > r->width) || (r->width >
> > > > config->max_width)) {
> > > > +       if ((config->min_width > r->width) ||
> > > > +           (!config->max_fb_width && r->width >
> > > > config->max_width) ||
> > > > +           (config->max_fb_width && r->width >
> > > > config->max_fb_width)) {
> > > >                 DRM_DEBUG_KMS("bad framebuffer width %d, should
> > > > be >= %d && <= %d\n",
> > > > -                         r->width, config->min_width,
> > > > config->max_width);
> > > > +                         r->width, config->min_width,
> > > > config->max_fb_width ?
> > > > +                         config->max_fb_width : config->max_width);
> > > >                 return ERR_PTR(-EINVAL);
> > > >         }
> > > > -       if ((config->min_height > r->height) || (r->height >
> > > > config->max_height)) {
> > > > +       if ((config->min_height > r->height) ||
> > > > +           (!config->max_fb_height && r->height >
> > > > config->max_height) ||
> > > > +           (config->max_fb_height && r->height >
> > > > config->max_fb_height)) {
> > > >                 DRM_DEBUG_KMS("bad framebuffer height %d, should
> > > > be >= %d && <= %d\n",
> > > > -                         r->height, config->min_height,
> > > > config->max_height);
> > > > +                         r->height, config->min_height,
> > > > config->max_fb_height ?
> > > > +                         config->max_fb_height :
> > > > config->max_height);
> > > >                 return ERR_PTR(-EINVAL);
> > > >         }
> > > > 
> > > > and in the driver :
> > > > 
> > > > +       drm->mode_config.max_width = 4096;
> > > > +       drm->mode_config.max_height = 3840;
> > > > +       drm->mode_config.max_fb_width = 16384;
> > > > +       drm->mode_config.max_fb_height = 8192;
> > > > 
> > > > With this I leave the mode filtering intact.
> > > 
> > > Not enough. See
> > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connector_helper_funcs
> > > and scroll down to mode_valid. You need to filter modes both in the
> > > detect paths, and the atomic_check paths.
> > > 
> > > Detect is explicitly filtered out, but atomic_check was only
> > > implicitly filtered, through the max fb size checks. Ok, you could
> > > light up a mode that's bigger than max fb, but in practice, no
> > > userspace ever did that.
> 
> Daniel, MSM and few other vendor hardware have upscale blocks where the
> driver can expose fb sizes smaller than
> the mode resolution and use h/w upscaling to fill the screen. This would
> optimize the fetch bandwidth.
> 
> But with your code we're missing crucial
> > > validation now, and userspace could fall over that. What I think we
> > > need is to add mode filter against mode_config.max_width/height in
> > > drm_atomic_helper_check_modeset(). Probably best to stuff that into
> > > the mode_valid() function.
> > 
> Agreed! Since the above patch from Niel is taking care of cases where
> max/min fb values
> are not set, by checking against the original max/min values, can we
> separate out this
> core change from the driver level mode_valid changes? If Niel couldn't find
> the time, I can
> repost the above change.

Sure, I think Neil wouldn't mind if you take this over and get it ready
for merging. Just need to make sure we're not leaving any validation gaps
in core/helper code.
-Daniel

> 
> Thanks and Regards,
> Jeykumar S.
> 
> > Ok I understood now, thanks for pointer, I'll try to add this.
> > 
> > Neil
> > 
> > > 
> > > Cheers, Daniel
> > > > 
> > > > Neil
> > > > 
> > > > 
> > > > > -Daniel
> > > > > 
> > > > > > 
> > > > > > Neil
> > > > > > 
> > > > > > > 
> > > > > > > Bunch of igt to make sure we're not missing anything
> > > > > > > would be sweet on
> > > > > > > top, e.g. e.g. trying to set a mode over the limit
> > > > > > > and making sure it
> > > > > > > fails.
> > > > > > > 
> > > > > > > Cheers, Daniel
> > > > > > > 
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> > > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > b/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > index d344312..2e29968 100644
> > > > > > > > --- a/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > +++ b/drivers/gpu/drm/meson/meson_drv.c
> > > > > > > > @@ -243,8 +243,8 @@ static int
> > > > > > > > meson_drv_bind_master(struct device *dev, bool
> > > > > > > > has_components)
> > > > > > > >              goto free_drm;
> > > > > > > > 
> > > > > > > >      drm_mode_config_init(drm);
> > > > > > > > -    drm->mode_config.max_width = 3840;
> > > > > > > > -    drm->mode_config.max_height = 2160;
> > > > > > > > +    drm->mode_config.max_width = 16384;
> > > > > > > > +    drm->mode_config.max_height = 8192;
> > > > > > > >      drm->mode_config.funcs = &meson_mode_config_funcs;
> > > > > > > > 
> > > > > > > >      /* Hardware Initialization */
> > > > > > > > --
> > > > > > > > 2.7.4
> > > > > > > > 
> > > > > > > > _______________________________________________
> > > > > > > > dri-devel mailing list
> > > > > > > > dri-devel@lists.freedesktop.org
> > > > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > > > > 
> > > > > > 
> > > > > > _______________________________________________
> > > > > > dri-devel mailing list
> > > > > > dri-devel@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > > 
> > > > > 
> > > > > 
> > > > 
> > > 
> > > 
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Jeykumar S

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] drm/meson: fix max mode_config height/width

[Jeykumar Sankaran]

On 2019-10-09 03:47, Daniel Vetter wrote:
> On Tue, Sep 24, 2019 at 10:28:48AM -0700, Jeykumar Sankaran wrote:
>> Reviving this thread from the context of the below conversion:
>> 
>> 
> https://lore.kernel.org/linux-arm-msm/db26145b-3f64-a334-f698-76f972332881
> @baylibre.com/T/#u
>> 
>> On 2018-10-05 01:19, Neil Armstrong wrote:
>> > On 05/10/2018 09:58, Daniel Vetter wrote:
>> > > On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong
>> > > <narmstrong@baylibre.com> wrote:
>> > > >
>> >
>> > [...]
>> >
>> > > > OK, won't this be enough ?
>> > > > --- a/include/drm/drm_mode_config.h
>> > > > +++ b/include/drm/drm_mode_config.h
>> > > > @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>> > > >   * @min_height: minimum fb pixel height on this device
>> > > >   * @max_width: maximum fb pixel width on this device
>> > > >   * @max_height: maximum fb pixel height on this device
>> > > > + * @max_fb_width: maximum fb buffer width if differs from
> max_width
>> > > > + * @max_fb_height: maximum fb buffer height if differs from
>> > > > max_height
>> > > >   * @funcs: core driver provided mode setting functions
>> > > >   * @fb_base: base address of the framebuffer
>> > > >   * @poll_enabled: track polling support for this device
>> > > > @@ -508,6 +510,7 @@ struct drm_mode_config {
>> > > >
>> > > >         int min_width, min_height;
>> > > >         int max_width, max_height;
>> > > > +       int max_fb_width, max_fb_height;
>> > > >         const struct drm_mode_config_funcs *funcs;
>> > > >         resource_size_t fb_base;
>> > > >
>> > > > --- a/drivers/gpu/drm/drm_framebuffer.c
>> > > > +++ b/drivers/gpu/drm/drm_framebuffer.c
>> > > > @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct
>> > > > drm_device *dev,
>> > > >                 return ERR_PTR(-EINVAL);
>> > > >         }
>> > > >
>> > > > -       if ((config->min_width > r->width) || (r->width >
>> > > > config->max_width)) {
>> > > > +       if ((config->min_width > r->width) ||
>> > > > +           (!config->max_fb_width && r->width >
>> > > > config->max_width) ||
>> > > > +           (config->max_fb_width && r->width >
>> > > > config->max_fb_width)) {
>> > > >                 DRM_DEBUG_KMS("bad framebuffer width %d, should
>> > > > be >= %d && <= %d\n",
>> > > > -                         r->width, config->min_width,
>> > > > config->max_width);
>> > > > +                         r->width, config->min_width,
>> > > > config->max_fb_width ?
>> > > > +                         config->max_fb_width :
> config->max_width);
>> > > >                 return ERR_PTR(-EINVAL);
>> > > >         }
>> > > > -       if ((config->min_height > r->height) || (r->height >
>> > > > config->max_height)) {
>> > > > +       if ((config->min_height > r->height) ||
>> > > > +           (!config->max_fb_height && r->height >
>> > > > config->max_height) ||
>> > > > +           (config->max_fb_height && r->height >
>> > > > config->max_fb_height)) {
>> > > >                 DRM_DEBUG_KMS("bad framebuffer height %d, should
>> > > > be >= %d && <= %d\n",
>> > > > -                         r->height, config->min_height,
>> > > > config->max_height);
>> > > > +                         r->height, config->min_height,
>> > > > config->max_fb_height ?
>> > > > +                         config->max_fb_height :
>> > > > config->max_height);
>> > > >                 return ERR_PTR(-EINVAL);
>> > > >         }
>> > > >
>> > > > and in the driver :
>> > > >
>> > > > +       drm->mode_config.max_width = 4096;
>> > > > +       drm->mode_config.max_height = 3840;
>> > > > +       drm->mode_config.max_fb_width = 16384;
>> > > > +       drm->mode_config.max_fb_height = 8192;
>> > > >
>> > > > With this I leave the mode filtering intact.
>> > >
>> > > Not enough. See
>> > >
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connec
> tor_helper_funcs
>> > > and scroll down to mode_valid. You need to filter modes both in the
>> > > detect paths, and the atomic_check paths.
>> > >
>> > > Detect is explicitly filtered out, but atomic_check was only
>> > > implicitly filtered, through the max fb size checks. Ok, you could
>> > > light up a mode that's bigger than max fb, but in practice, no
>> > > userspace ever did that.
>> 
>> Daniel, MSM and few other vendor hardware have upscale blocks where 
>> the
>> driver can expose fb sizes smaller than
>> the mode resolution and use h/w upscaling to fill the screen. This 
>> would
>> optimize the fetch bandwidth.
>> 
>> But with your code we're missing crucial
>> > > validation now, and userspace could fall over that. What I think we
>> > > need is to add mode filter against mode_config.max_width/height in
>> > > drm_atomic_helper_check_modeset(). Probably best to stuff that into
>> > > the mode_valid() function.
>> >
>> Agreed! Since the above patch from Niel is taking care of cases where
>> max/min fb values
>> are not set, by checking against the original max/min values, can we
>> separate out this
>> core change from the driver level mode_valid changes? If Niel couldn't
> find
>> the time, I can
>> repost the above change.
> 
> Sure, I think Neil wouldn't mind if you take this over and get it ready
> for merging. Just need to make sure we're not leaving any validation 
> gaps
> in core/helper code.
> -Daniel
> 
I guess you are a bit late for the party!

I did post the patch on the forum. The latest on the thread can be found 
here: https://lkml.org/lkml/2019/10/2/369

The basic concern is if FB limits are different (especially smaller) 
than the display (mode) limits, it
will break the existing user space, who are creating unscaled FB's out 
of exposed mode limits.

Thanks and Regards,
Jeykumar S.

>> 
>> Thanks and Regards,
>> Jeykumar S.
>> 
>> > Ok I understood now, thanks for pointer, I'll try to add this.
>> >
>> > Neil
>> >
>> > >
>> > > Cheers, Daniel
>> > > >
>> > > > Neil
>> > > >
>> > > >
>> > > > > -Daniel
>> > > > >
>> > > > > >
>> > > > > > Neil
>> > > > > >
>> > > > > > >
>> > > > > > > Bunch of igt to make sure we're not missing anything
>> > > > > > > would be sweet on
>> > > > > > > top, e.g. e.g. trying to set a mode over the limit
>> > > > > > > and making sure it
>> > > > > > > fails.
>> > > > > > >
>> > > > > > > Cheers, Daniel
>> > > > > > >
>> > > > > > > > ---
>> > > > > > > >  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>> > > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
>> > > > > > > >
>> > > > > > > > diff --git a/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > b/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > index d344312..2e29968 100644
>> > > > > > > > --- a/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > +++ b/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > @@ -243,8 +243,8 @@ static int
>> > > > > > > > meson_drv_bind_master(struct device *dev, bool
>> > > > > > > > has_components)
>> > > > > > > >              goto free_drm;
>> > > > > > > >
>> > > > > > > >      drm_mode_config_init(drm);
>> > > > > > > > -    drm->mode_config.max_width = 3840;
>> > > > > > > > -    drm->mode_config.max_height = 2160;
>> > > > > > > > +    drm->mode_config.max_width = 16384;
>> > > > > > > > +    drm->mode_config.max_height = 8192;
>> > > > > > > >      drm->mode_config.funcs = &meson_mode_config_funcs;
>> > > > > > > >
>> > > > > > > >      /* Hardware Initialization */
>> > > > > > > > --
>> > > > > > > > 2.7.4
>> > > > > > > >
>> > > > > > > > _______________________________________________
>> > > > > > > > dri-devel mailing list
>> > > > > > > > dri-devel@lists.freedesktop.org
>> > > > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> > > > > > >
>> > > > > >
>> > > > > > _______________________________________________
>> > > > > > dri-devel mailing list
>> > > > > > dri-devel@lists.freedesktop.org
>> > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> > > > >
>> > > > >
>> > > > >
>> > > >
>> > >
>> > >
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> 
>> --
>> Jeykumar S

-- 
Jeykumar S

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] drm/meson: fix max mode_config height/width

[Jeykumar Sankaran]

On 2019-10-09 03:47, Daniel Vetter wrote:
> On Tue, Sep 24, 2019 at 10:28:48AM -0700, Jeykumar Sankaran wrote:
>> Reviving this thread from the context of the below conversion:
>> 
>> 
> https://lore.kernel.org/linux-arm-msm/db26145b-3f64-a334-f698-76f972332881
> @baylibre.com/T/#u
>> 
>> On 2018-10-05 01:19, Neil Armstrong wrote:
>> > On 05/10/2018 09:58, Daniel Vetter wrote:
>> > > On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong
>> > > <narmstrong@baylibre.com> wrote:
>> > > >
>> >
>> > [...]
>> >
>> > > > OK, won't this be enough ?
>> > > > --- a/include/drm/drm_mode_config.h
>> > > > +++ b/include/drm/drm_mode_config.h
>> > > > @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>> > > >   * @min_height: minimum fb pixel height on this device
>> > > >   * @max_width: maximum fb pixel width on this device
>> > > >   * @max_height: maximum fb pixel height on this device
>> > > > + * @max_fb_width: maximum fb buffer width if differs from
> max_width
>> > > > + * @max_fb_height: maximum fb buffer height if differs from
>> > > > max_height
>> > > >   * @funcs: core driver provided mode setting functions
>> > > >   * @fb_base: base address of the framebuffer
>> > > >   * @poll_enabled: track polling support for this device
>> > > > @@ -508,6 +510,7 @@ struct drm_mode_config {
>> > > >
>> > > >         int min_width, min_height;
>> > > >         int max_width, max_height;
>> > > > +       int max_fb_width, max_fb_height;
>> > > >         const struct drm_mode_config_funcs *funcs;
>> > > >         resource_size_t fb_base;
>> > > >
>> > > > --- a/drivers/gpu/drm/drm_framebuffer.c
>> > > > +++ b/drivers/gpu/drm/drm_framebuffer.c
>> > > > @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct
>> > > > drm_device *dev,
>> > > >                 return ERR_PTR(-EINVAL);
>> > > >         }
>> > > >
>> > > > -       if ((config->min_width > r->width) || (r->width >
>> > > > config->max_width)) {
>> > > > +       if ((config->min_width > r->width) ||
>> > > > +           (!config->max_fb_width && r->width >
>> > > > config->max_width) ||
>> > > > +           (config->max_fb_width && r->width >
>> > > > config->max_fb_width)) {
>> > > >                 DRM_DEBUG_KMS("bad framebuffer width %d, should
>> > > > be >= %d && <= %d\n",
>> > > > -                         r->width, config->min_width,
>> > > > config->max_width);
>> > > > +                         r->width, config->min_width,
>> > > > config->max_fb_width ?
>> > > > +                         config->max_fb_width :
> config->max_width);
>> > > >                 return ERR_PTR(-EINVAL);
>> > > >         }
>> > > > -       if ((config->min_height > r->height) || (r->height >
>> > > > config->max_height)) {
>> > > > +       if ((config->min_height > r->height) ||
>> > > > +           (!config->max_fb_height && r->height >
>> > > > config->max_height) ||
>> > > > +           (config->max_fb_height && r->height >
>> > > > config->max_fb_height)) {
>> > > >                 DRM_DEBUG_KMS("bad framebuffer height %d, should
>> > > > be >= %d && <= %d\n",
>> > > > -                         r->height, config->min_height,
>> > > > config->max_height);
>> > > > +                         r->height, config->min_height,
>> > > > config->max_fb_height ?
>> > > > +                         config->max_fb_height :
>> > > > config->max_height);
>> > > >                 return ERR_PTR(-EINVAL);
>> > > >         }
>> > > >
>> > > > and in the driver :
>> > > >
>> > > > +       drm->mode_config.max_width = 4096;
>> > > > +       drm->mode_config.max_height = 3840;
>> > > > +       drm->mode_config.max_fb_width = 16384;
>> > > > +       drm->mode_config.max_fb_height = 8192;
>> > > >
>> > > > With this I leave the mode filtering intact.
>> > >
>> > > Not enough. See
>> > >
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connec
> tor_helper_funcs
>> > > and scroll down to mode_valid. You need to filter modes both in the
>> > > detect paths, and the atomic_check paths.
>> > >
>> > > Detect is explicitly filtered out, but atomic_check was only
>> > > implicitly filtered, through the max fb size checks. Ok, you could
>> > > light up a mode that's bigger than max fb, but in practice, no
>> > > userspace ever did that.
>> 
>> Daniel, MSM and few other vendor hardware have upscale blocks where 
>> the
>> driver can expose fb sizes smaller than
>> the mode resolution and use h/w upscaling to fill the screen. This 
>> would
>> optimize the fetch bandwidth.
>> 
>> But with your code we're missing crucial
>> > > validation now, and userspace could fall over that. What I think we
>> > > need is to add mode filter against mode_config.max_width/height in
>> > > drm_atomic_helper_check_modeset(). Probably best to stuff that into
>> > > the mode_valid() function.
>> >
>> Agreed! Since the above patch from Niel is taking care of cases where
>> max/min fb values
>> are not set, by checking against the original max/min values, can we
>> separate out this
>> core change from the driver level mode_valid changes? If Niel couldn't
> find
>> the time, I can
>> repost the above change.
> 
> Sure, I think Neil wouldn't mind if you take this over and get it ready
> for merging. Just need to make sure we're not leaving any validation 
> gaps
> in core/helper code.
> -Daniel
> 
I guess you are a bit late for the party!

I did post the patch on the forum. The latest on the thread can be found 
here: https://lkml.org/lkml/2019/10/2/369

The basic concern is if FB limits are different (especially smaller) 
than the display (mode) limits, it
will break the existing user space, who are creating unscaled FB's out 
of exposed mode limits.

Thanks and Regards,
Jeykumar S.

>> 
>> Thanks and Regards,
>> Jeykumar S.
>> 
>> > Ok I understood now, thanks for pointer, I'll try to add this.
>> >
>> > Neil
>> >
>> > >
>> > > Cheers, Daniel
>> > > >
>> > > > Neil
>> > > >
>> > > >
>> > > > > -Daniel
>> > > > >
>> > > > > >
>> > > > > > Neil
>> > > > > >
>> > > > > > >
>> > > > > > > Bunch of igt to make sure we're not missing anything
>> > > > > > > would be sweet on
>> > > > > > > top, e.g. e.g. trying to set a mode over the limit
>> > > > > > > and making sure it
>> > > > > > > fails.
>> > > > > > >
>> > > > > > > Cheers, Daniel
>> > > > > > >
>> > > > > > > > ---
>> > > > > > > >  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>> > > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
>> > > > > > > >
>> > > > > > > > diff --git a/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > b/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > index d344312..2e29968 100644
>> > > > > > > > --- a/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > +++ b/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > @@ -243,8 +243,8 @@ static int
>> > > > > > > > meson_drv_bind_master(struct device *dev, bool
>> > > > > > > > has_components)
>> > > > > > > >              goto free_drm;
>> > > > > > > >
>> > > > > > > >      drm_mode_config_init(drm);
>> > > > > > > > -    drm->mode_config.max_width = 3840;
>> > > > > > > > -    drm->mode_config.max_height = 2160;
>> > > > > > > > +    drm->mode_config.max_width = 16384;
>> > > > > > > > +    drm->mode_config.max_height = 8192;
>> > > > > > > >      drm->mode_config.funcs = &meson_mode_config_funcs;
>> > > > > > > >
>> > > > > > > >      /* Hardware Initialization */
>> > > > > > > > --
>> > > > > > > > 2.7.4
>> > > > > > > >
>> > > > > > > > _______________________________________________
>> > > > > > > > dri-devel mailing list
>> > > > > > > > dri-devel@lists.freedesktop.org
>> > > > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> > > > > > >
>> > > > > >
>> > > > > > _______________________________________________
>> > > > > > dri-devel mailing list
>> > > > > > dri-devel@lists.freedesktop.org
>> > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> > > > >
>> > > > >
>> > > > >
>> > > >
>> > >
>> > >
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> 
>> --
>> Jeykumar S

-- 
Jeykumar S

Re: [PATCH] drm/meson: fix max mode_config height/width

[Jeykumar Sankaran]

On 2019-10-09 03:47, Daniel Vetter wrote:
> On Tue, Sep 24, 2019 at 10:28:48AM -0700, Jeykumar Sankaran wrote:
>> Reviving this thread from the context of the below conversion:
>> 
>> 
> https://lore.kernel.org/linux-arm-msm/db26145b-3f64-a334-f698-76f972332881
> @baylibre.com/T/#u
>> 
>> On 2018-10-05 01:19, Neil Armstrong wrote:
>> > On 05/10/2018 09:58, Daniel Vetter wrote:
>> > > On Fri, Oct 5, 2018 at 9:39 AM Neil Armstrong
>> > > <narmstrong@baylibre.com> wrote:
>> > > >
>> >
>> > [...]
>> >
>> > > > OK, won't this be enough ?
>> > > > --- a/include/drm/drm_mode_config.h
>> > > > +++ b/include/drm/drm_mode_config.h
>> > > > @@ -333,6 +333,8 @@ struct drm_mode_config_funcs {
>> > > >   * @min_height: minimum fb pixel height on this device
>> > > >   * @max_width: maximum fb pixel width on this device
>> > > >   * @max_height: maximum fb pixel height on this device
>> > > > + * @max_fb_width: maximum fb buffer width if differs from
> max_width
>> > > > + * @max_fb_height: maximum fb buffer height if differs from
>> > > > max_height
>> > > >   * @funcs: core driver provided mode setting functions
>> > > >   * @fb_base: base address of the framebuffer
>> > > >   * @poll_enabled: track polling support for this device
>> > > > @@ -508,6 +510,7 @@ struct drm_mode_config {
>> > > >
>> > > >         int min_width, min_height;
>> > > >         int max_width, max_height;
>> > > > +       int max_fb_width, max_fb_height;
>> > > >         const struct drm_mode_config_funcs *funcs;
>> > > >         resource_size_t fb_base;
>> > > >
>> > > > --- a/drivers/gpu/drm/drm_framebuffer.c
>> > > > +++ b/drivers/gpu/drm/drm_framebuffer.c
>> > > > @@ -283,14 +283,20 @@ drm_internal_framebuffer_create(struct
>> > > > drm_device *dev,
>> > > >                 return ERR_PTR(-EINVAL);
>> > > >         }
>> > > >
>> > > > -       if ((config->min_width > r->width) || (r->width >
>> > > > config->max_width)) {
>> > > > +       if ((config->min_width > r->width) ||
>> > > > +           (!config->max_fb_width && r->width >
>> > > > config->max_width) ||
>> > > > +           (config->max_fb_width && r->width >
>> > > > config->max_fb_width)) {
>> > > >                 DRM_DEBUG_KMS("bad framebuffer width %d, should
>> > > > be >= %d && <= %d\n",
>> > > > -                         r->width, config->min_width,
>> > > > config->max_width);
>> > > > +                         r->width, config->min_width,
>> > > > config->max_fb_width ?
>> > > > +                         config->max_fb_width :
> config->max_width);
>> > > >                 return ERR_PTR(-EINVAL);
>> > > >         }
>> > > > -       if ((config->min_height > r->height) || (r->height >
>> > > > config->max_height)) {
>> > > > +       if ((config->min_height > r->height) ||
>> > > > +           (!config->max_fb_height && r->height >
>> > > > config->max_height) ||
>> > > > +           (config->max_fb_height && r->height >
>> > > > config->max_fb_height)) {
>> > > >                 DRM_DEBUG_KMS("bad framebuffer height %d, should
>> > > > be >= %d && <= %d\n",
>> > > > -                         r->height, config->min_height,
>> > > > config->max_height);
>> > > > +                         r->height, config->min_height,
>> > > > config->max_fb_height ?
>> > > > +                         config->max_fb_height :
>> > > > config->max_height);
>> > > >                 return ERR_PTR(-EINVAL);
>> > > >         }
>> > > >
>> > > > and in the driver :
>> > > >
>> > > > +       drm->mode_config.max_width = 4096;
>> > > > +       drm->mode_config.max_height = 3840;
>> > > > +       drm->mode_config.max_fb_width = 16384;
>> > > > +       drm->mode_config.max_fb_height = 8192;
>> > > >
>> > > > With this I leave the mode filtering intact.
>> > >
>> > > Not enough. See
>> > >
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_connec
> tor_helper_funcs
>> > > and scroll down to mode_valid. You need to filter modes both in the
>> > > detect paths, and the atomic_check paths.
>> > >
>> > > Detect is explicitly filtered out, but atomic_check was only
>> > > implicitly filtered, through the max fb size checks. Ok, you could
>> > > light up a mode that's bigger than max fb, but in practice, no
>> > > userspace ever did that.
>> 
>> Daniel, MSM and few other vendor hardware have upscale blocks where 
>> the
>> driver can expose fb sizes smaller than
>> the mode resolution and use h/w upscaling to fill the screen. This 
>> would
>> optimize the fetch bandwidth.
>> 
>> But with your code we're missing crucial
>> > > validation now, and userspace could fall over that. What I think we
>> > > need is to add mode filter against mode_config.max_width/height in
>> > > drm_atomic_helper_check_modeset(). Probably best to stuff that into
>> > > the mode_valid() function.
>> >
>> Agreed! Since the above patch from Niel is taking care of cases where
>> max/min fb values
>> are not set, by checking against the original max/min values, can we
>> separate out this
>> core change from the driver level mode_valid changes? If Niel couldn't
> find
>> the time, I can
>> repost the above change.
> 
> Sure, I think Neil wouldn't mind if you take this over and get it ready
> for merging. Just need to make sure we're not leaving any validation 
> gaps
> in core/helper code.
> -Daniel
> 
I guess you are a bit late for the party!

I did post the patch on the forum. The latest on the thread can be found 
here: https://lkml.org/lkml/2019/10/2/369

The basic concern is if FB limits are different (especially smaller) 
than the display (mode) limits, it
will break the existing user space, who are creating unscaled FB's out 
of exposed mode limits.

Thanks and Regards,
Jeykumar S.

>> 
>> Thanks and Regards,
>> Jeykumar S.
>> 
>> > Ok I understood now, thanks for pointer, I'll try to add this.
>> >
>> > Neil
>> >
>> > >
>> > > Cheers, Daniel
>> > > >
>> > > > Neil
>> > > >
>> > > >
>> > > > > -Daniel
>> > > > >
>> > > > > >
>> > > > > > Neil
>> > > > > >
>> > > > > > >
>> > > > > > > Bunch of igt to make sure we're not missing anything
>> > > > > > > would be sweet on
>> > > > > > > top, e.g. e.g. trying to set a mode over the limit
>> > > > > > > and making sure it
>> > > > > > > fails.
>> > > > > > >
>> > > > > > > Cheers, Daniel
>> > > > > > >
>> > > > > > > > ---
>> > > > > > > >  drivers/gpu/drm/meson/meson_drv.c | 4 ++--
>> > > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
>> > > > > > > >
>> > > > > > > > diff --git a/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > b/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > index d344312..2e29968 100644
>> > > > > > > > --- a/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > +++ b/drivers/gpu/drm/meson/meson_drv.c
>> > > > > > > > @@ -243,8 +243,8 @@ static int
>> > > > > > > > meson_drv_bind_master(struct device *dev, bool
>> > > > > > > > has_components)
>> > > > > > > >              goto free_drm;
>> > > > > > > >
>> > > > > > > >      drm_mode_config_init(drm);
>> > > > > > > > -    drm->mode_config.max_width = 3840;
>> > > > > > > > -    drm->mode_config.max_height = 2160;
>> > > > > > > > +    drm->mode_config.max_width = 16384;
>> > > > > > > > +    drm->mode_config.max_height = 8192;
>> > > > > > > >      drm->mode_config.funcs = &meson_mode_config_funcs;
>> > > > > > > >
>> > > > > > > >      /* Hardware Initialization */
>> > > > > > > > --
>> > > > > > > > 2.7.4
>> > > > > > > >
>> > > > > > > > _______________________________________________
>> > > > > > > > dri-devel mailing list
>> > > > > > > > dri-devel@lists.freedesktop.org
>> > > > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> > > > > > >
>> > > > > >
>> > > > > > _______________________________________________
>> > > > > > dri-devel mailing list
>> > > > > > dri-devel@lists.freedesktop.org
>> > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> > > > >
>> > > > >
>> > > > >
>> > > >
>> > >
>> > >
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> 
>> --
>> Jeykumar S

-- 
Jeykumar S

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel