dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm: Fix for invalid pruning of modes in dual display cases
@ 2016-12-13 10:16 Vidya Srinivas
  2016-12-13 10:28 ` [Intel-gfx] " Jani Nikula
  2016-12-13 10:46 ` Chris Wilson
  0 siblings, 2 replies; 6+ messages in thread
From: Vidya Srinivas @ 2016-12-13 10:16 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Vidya Srinivas

Currently in dual display connected boot scenarios, minimum of the resolutions
is taken for fb width and height as reference. Based on this resolution, other
modes are pruned.

Example Scenario: If DSI mode is 2560x1440 and HDMI is 1920x1080, during the probing
the fb width and height is set to max 1920x1080 and the DSI mode gets pruned as it is
more than the reference. As a result, there is no DSI display.
Patch fixes this issue by taking the max of the resolutions and creating the fb
based on the same.

Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index e934b54..6afc06f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1482,8 +1482,8 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
 	sizes.surface_depth = 24;
 	sizes.surface_bpp = 32;
-	sizes.fb_width = (unsigned)-1;
-	sizes.fb_height = (unsigned)-1;
+	sizes.fb_width = 0;
+	sizes.fb_height = 0;
 
 	/* if driver picks 8 or 16 by default use that
 	   for both depth/bpp */
@@ -1560,9 +1560,9 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 		}
 
 		if (lasth)
-			sizes.fb_width  = min_t(u32, desired_mode->hdisplay + x, sizes.fb_width);
+			sizes.fb_width  = max_t(u32, desired_mode->hdisplay + x, sizes.fb_width);
 		if (lastv)
-			sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
+			sizes.fb_height = max_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
 	}
 
 	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: Fix for invalid pruning of modes in dual display cases
  2016-12-13 10:16 [PATCH] drm: Fix for invalid pruning of modes in dual display cases Vidya Srinivas
@ 2016-12-13 10:28 ` Jani Nikula
  2016-12-13 10:46 ` Chris Wilson
  1 sibling, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2016-12-13 10:28 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Vidya Srinivas

On Tue, 13 Dec 2016, Vidya Srinivas <vidya.srinivas@intel.com> wrote:
> Currently in dual display connected boot scenarios, minimum of the resolutions
> is taken for fb width and height as reference. Based on this resolution, other
> modes are pruned.
>
> Example Scenario: If DSI mode is 2560x1440 and HDMI is 1920x1080, during the probing
> the fb width and height is set to max 1920x1080 and the DSI mode gets pruned as it is
> more than the reference. As a result, there is no DSI display.
> Patch fixes this issue by taking the max of the resolutions and creating the fb
> based on the same.
>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index e934b54..6afc06f 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1482,8 +1482,8 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  	memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
>  	sizes.surface_depth = 24;
>  	sizes.surface_bpp = 32;
> -	sizes.fb_width = (unsigned)-1;
> -	sizes.fb_height = (unsigned)-1;
> +	sizes.fb_width = 0;
> +	sizes.fb_height = 0;
>  
>  	/* if driver picks 8 or 16 by default use that
>  	   for both depth/bpp */
> @@ -1560,9 +1560,9 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  		}
>  
>  		if (lasth)
> -			sizes.fb_width  = min_t(u32, desired_mode->hdisplay + x, sizes.fb_width);
> +			sizes.fb_width  = max_t(u32, desired_mode->hdisplay + x, sizes.fb_width);
>  		if (lastv)
> -			sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
> +			sizes.fb_height = max_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
>  	}
>  
>  	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {

Not commenting on the main change itself, but just as a reminder, these
checks will need updating too.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm: Fix for invalid pruning of modes in dual display cases
  2016-12-13 10:16 [PATCH] drm: Fix for invalid pruning of modes in dual display cases Vidya Srinivas
  2016-12-13 10:28 ` [Intel-gfx] " Jani Nikula
@ 2016-12-13 10:46 ` Chris Wilson
  2016-12-13 14:52   ` [Intel-gfx] " Jani Nikula
  1 sibling, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-12-13 10:46 UTC (permalink / raw)
  To: Vidya Srinivas; +Cc: intel-gfx, dri-devel

On Tue, Dec 13, 2016 at 03:46:54PM +0530, Vidya Srinivas wrote:
> Currently in dual display connected boot scenarios, minimum of the resolutions
> is taken for fb width and height as reference. Based on this resolution, other
> modes are pruned.
> 
> Example Scenario: If DSI mode is 2560x1440 and HDMI is 1920x1080, during the probing
> the fb width and height is set to max 1920x1080 and the DSI mode gets pruned as it is
> more than the reference. As a result, there is no DSI display.
> Patch fixes this issue by taking the max of the resolutions and creating the fb
> based on the same.

On the other hand, the viewable content is defined by the smaller mode.
If that is the only output visible at panic time, you don't want that
information lost due to it being invisible.

This is only used for fbcon, which has to be the lowest common
denominator. Any actual application can set their own modes and fb.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: Fix for invalid pruning of modes in dual display cases
  2016-12-13 10:46 ` Chris Wilson
@ 2016-12-13 14:52   ` Jani Nikula
  2016-12-13 15:46     ` Ville Syrjälä
  0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2016-12-13 14:52 UTC (permalink / raw)
  To: Chris Wilson, Vidya Srinivas; +Cc: intel-gfx, Syrjala, Ville, dri-devel

On Tue, 13 Dec 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Dec 13, 2016 at 03:46:54PM +0530, Vidya Srinivas wrote:
>> Currently in dual display connected boot scenarios, minimum of the resolutions
>> is taken for fb width and height as reference. Based on this resolution, other
>> modes are pruned.
>> 
>> Example Scenario: If DSI mode is 2560x1440 and HDMI is 1920x1080, during the probing
>> the fb width and height is set to max 1920x1080 and the DSI mode gets pruned as it is
>> more than the reference. As a result, there is no DSI display.
>> Patch fixes this issue by taking the max of the resolutions and creating the fb
>> based on the same.
>
> On the other hand, the viewable content is defined by the smaller mode.
> If that is the only output visible at panic time, you don't want that
> information lost due to it being invisible.
>
> This is only used for fbcon, which has to be the lowest common
> denominator. Any actual application can set their own modes and fb.

Do we fail to take over non-native modes with scaling on DSI then?
Ville?

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH] drm: Fix for invalid pruning of modes in dual display cases
  2016-12-13 14:52   ` [Intel-gfx] " Jani Nikula
@ 2016-12-13 15:46     ` Ville Syrjälä
  2017-01-16 10:13       ` Srinivas, Vidya
  0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2016-12-13 15:46 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Syrjala, Ville, Vidya Srinivas, dri-devel

On Tue, Dec 13, 2016 at 04:52:01PM +0200, Jani Nikula wrote:
> On Tue, 13 Dec 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Tue, Dec 13, 2016 at 03:46:54PM +0530, Vidya Srinivas wrote:
> >> Currently in dual display connected boot scenarios, minimum of the resolutions
> >> is taken for fb width and height as reference. Based on this resolution, other
> >> modes are pruned.
> >> 
> >> Example Scenario: If DSI mode is 2560x1440 and HDMI is 1920x1080, during the probing
> >> the fb width and height is set to max 1920x1080 and the DSI mode gets pruned as it is
> >> more than the reference. As a result, there is no DSI display.
> >> Patch fixes this issue by taking the max of the resolutions and creating the fb
> >> based on the same.
> >
> > On the other hand, the viewable content is defined by the smaller mode.
> > If that is the only output visible at panic time, you don't want that
> > information lost due to it being invisible.
> >
> > This is only used for fbcon, which has to be the lowest common
> > denominator. Any actual application can set their own modes and fb.
> 
> Do we fail to take over non-native modes with scaling on DSI then?
> Ville?

I believe we are taking the panel fitter into account when we
reconstruct the BIOS fb. So in case the BIOS is upscaling we'll
come up with some less than screen size framebuffer. The fb
helper will want to use the native mode of the panel however,
so at some point we'll drop the BIOS fb (as it's too small)
and allocate a new one.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm: Fix for invalid pruning of modes in dual display cases
  2016-12-13 15:46     ` Ville Syrjälä
@ 2017-01-16 10:13       ` Srinivas, Vidya
  0 siblings, 0 replies; 6+ messages in thread
From: Srinivas, Vidya @ 2017-01-16 10:13 UTC (permalink / raw)
  To: Ville Syrjälä, Jani Nikula
  Cc: intel-gfx@lists.freedesktop.org, Syrjala, Ville,
	dri-devel@lists.freedesktop.org



> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Tuesday, December 13, 2016 9:17 PM
> To: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>; Srinivas, Vidya
> <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org; Syrjala, Ville
> <ville.syrjala@intel.com>; dri-devel@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm: Fix for invalid pruning of modes in dual
> display cases
> 
> On Tue, Dec 13, 2016 at 04:52:01PM +0200, Jani Nikula wrote:
> > On Tue, 13 Dec 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > On Tue, Dec 13, 2016 at 03:46:54PM +0530, Vidya Srinivas wrote:
> > >> Currently in dual display connected boot scenarios, minimum of the
> > >> resolutions is taken for fb width and height as reference. Based on
> > >> this resolution, other modes are pruned.
> > >>
> > >> Example Scenario: If DSI mode is 2560x1440 and HDMI is 1920x1080,
> > >> during the probing the fb width and height is set to max 1920x1080
> > >> and the DSI mode gets pruned as it is more than the reference. As a
> result, there is no DSI display.
> > >> Patch fixes this issue by taking the max of the resolutions and
> > >> creating the fb based on the same.
> > >
> > > On the other hand, the viewable content is defined by the smaller mode.
> > > If that is the only output visible at panic time, you don't want
> > > that information lost due to it being invisible.
> > >
> > > This is only used for fbcon, which has to be the lowest common
> > > denominator. Any actual application can set their own modes and fb.
> >
> > Do we fail to take over non-native modes with scaling on DSI then?
> > Ville?
> 
> I believe we are taking the panel fitter into account when we reconstruct the
> BIOS fb. So in case the BIOS is upscaling we'll come up with some less than
> screen size framebuffer. The fb helper will want to use the native mode of
> the panel however, so at some point we'll drop the BIOS fb (as it's too small)
> and allocate a new one.
We are not sure how to take this further. Can you please provide some inputs
for next steps on this? We will try to make the changes and test as per your
inputs/feedback.
> 
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-01-16 10:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-13 10:16 [PATCH] drm: Fix for invalid pruning of modes in dual display cases Vidya Srinivas
2016-12-13 10:28 ` [Intel-gfx] " Jani Nikula
2016-12-13 10:46 ` Chris Wilson
2016-12-13 14:52   ` [Intel-gfx] " Jani Nikula
2016-12-13 15:46     ` Ville Syrjälä
2017-01-16 10:13       ` Srinivas, Vidya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).