| Message ID | 20191031214839.27039-1-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/fbdev: Fallback to non tiled mode if all tiles not present | expand |
On Thu, Oct 31, 2019 at 02:48:39PM -0700, Manasi Navare wrote: > In case of tiled displays, if we hotplug just one connector, > fbcon currently just selects the preferred mode and if it is > tiled mode then that becomes a problem if rest of the tiles are > not present. > So in the fbdev driver on hotplug when we probe the client modeset, > we we dont find all the connectors for all tiles, then on a connector > with one tile, just fallback to the first available non tiled mode > to display over a single connector. > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Suggested-by: Dave Airlie <airlied@redhat.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Dave Airlie <airlied@redhat.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> Hm, should we mayb have a slight timeout first to wait for the 2nd connector? Otherwise lots of flickering going on when plugging in one of these screens ... -Daniel > --- > drivers/gpu/drm/drm_client_modeset.c | 29 ++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c > index 895b73f23079..e28a723587db 100644 > --- a/drivers/gpu/drm/drm_client_modeset.c > +++ b/drivers/gpu/drm/drm_client_modeset.c > @@ -114,6 +114,20 @@ drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc) > return NULL; > } > > +static struct drm_display_mode * > +drm_connector_fallback_non_tiled_mode(struct drm_connector *connector) > +{ > + struct drm_display_mode *mode; > + > + list_for_each_entry(mode, &connector->modes, head) { > + if (mode->hdisplay == connector->tile_h_size && > + mode->vdisplay == connector->tile_v_size) > + continue; > + return mode; > + } > + return NULL; > +} > + > static struct drm_display_mode * > drm_connector_has_preferred_mode(struct drm_connector *connector, int width, int height) > { > @@ -348,8 +362,17 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, > struct drm_connector *connector; > u64 conn_configured = 0; > int tile_pass = 0; > + int num_tiled_conns = 0; > int i; > > + for (i = 0; i < connector_count; i++) { > + connector = connectors[i]; > + if (!connector->has_tile) > + continue; > + > + num_tiled_conns ++; > + } > + > retry: > for (i = 0; i < connector_count; i++) { > connector = connectors[i]; > @@ -394,6 +417,12 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, > connector->base.id, connector->tile_group ? connector->tile_group->id : 0); > modes[i] = drm_connector_has_preferred_mode(connector, width, height); > } > + if (connector->has_tile && > + num_tiled_conns < connector->num_h_tile * connector->num_v_tile) { > + DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n", > + connector->base.id); > + modes[i] = drm_connector_fallback_non_tiled_mode(connector); > + } > /* No preferred modes, pick one off the list */ > if (!modes[i] && !list_empty(&connector->modes)) { > list_for_each_entry(modes[i], &connector->modes, head) > -- > 2.19.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Nov 04, 2019 at 07:48:26PM +1000, David Airlie wrote: > On Mon, Nov 4, 2019 at 7:18 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Thu, Oct 31, 2019 at 02:48:39PM -0700, Manasi Navare wrote: > > > In case of tiled displays, if we hotplug just one connector, > > > fbcon currently just selects the preferred mode and if it is > > > tiled mode then that becomes a problem if rest of the tiles are > > > not present. > > > So in the fbdev driver on hotplug when we probe the client modeset, > > > we we dont find all the connectors for all tiles, then on a connector > > > with one tile, just fallback to the first available non tiled mode > > > to display over a single connector. > > > > > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Suggested-by: Dave Airlie <airlied@redhat.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Dave Airlie <airlied@redhat.com> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > Hm, should we mayb have a slight timeout first to wait for the 2nd > > connector? Otherwise lots of flickering going on when plugging in one of > > these screens ... > > Not really, > > There are 3 scenarios with the multi-cable tiled monitors. and > non-resizeable fbdev. > > a) it's plugged in at boot. both cables are detected, fbdev gets a > full tiled mode. > b) it's not plugged in at boot, the user starts plugging it in, fbdev > was inited via the panel or another monitor. fbdev won't resize. > c) it's half plugged in at boot, then you get a non-tiled mode, and > fbdev can't resize to tiled anyways. > > Also plugging in one of these multi-cable monitors piecemeal is going > to take multiple seconds for the user to do physical cable plugging. Uh. I guess fbdev ftl, oh well. -Daniel
On Mon, Nov 04, 2019 at 07:48:26PM +1000, David Airlie wrote: > On Mon, Nov 4, 2019 at 7:18 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Thu, Oct 31, 2019 at 02:48:39PM -0700, Manasi Navare wrote: > > > In case of tiled displays, if we hotplug just one connector, > > > fbcon currently just selects the preferred mode and if it is > > > tiled mode then that becomes a problem if rest of the tiles are > > > not present. > > > So in the fbdev driver on hotplug when we probe the client modeset, > > > we we dont find all the connectors for all tiles, then on a connector > > > with one tile, just fallback to the first available non tiled mode > > > to display over a single connector. > > > > > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Suggested-by: Dave Airlie <airlied@redhat.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Dave Airlie <airlied@redhat.com> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > Hm, should we mayb have a slight timeout first to wait for the 2nd > > connector? Otherwise lots of flickering going on when plugging in one of > > these screens ... > > Not really, > > There are 3 scenarios with the multi-cable tiled monitors. and > non-resizeable fbdev. > > a) it's plugged in at boot. both cables are detected, fbdev gets a > full tiled mode. Yes this works as expected > b) it's not plugged in at boot, the user starts plugging it in, fbdev > was inited via the panel or another monitor. fbdev won't resize. > c) it's half plugged in at boot, then you get a non-tiled mode, and > fbdev can't resize to tiled anyways. In b and c, when its booted with only 1 cable connected and/or hotplugged only one cable after boot, I fallback to fisrt available non tiled mode, does that sound good? > > Also plugging in one of these multi-cable monitors piecemeal is going > to take multiple seconds for the user to do physical cable plugging. So still good with ignoring the second hotplug completely? However i donno where in the fb helper to stop passing the modeset down after the drm_fb_helper_hotplug_event() Any suggestions? Regards Manasi > > Dave. >
On Fri, 1 Nov 2019 at 07:46, Manasi Navare <manasi.d.navare@intel.com> wrote: > > In case of tiled displays, if we hotplug just one connector, > fbcon currently just selects the preferred mode and if it is > tiled mode then that becomes a problem if rest of the tiles are > not present. > So in the fbdev driver on hotplug when we probe the client modeset, > we we dont find all the connectors for all tiles, then on a connector > with one tile, just fallback to the first available non tiled mode > to display over a single connector. > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Suggested-by: Dave Airlie <airlied@redhat.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Dave Airlie <airlied@redhat.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/drm_client_modeset.c | 29 ++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c > index 895b73f23079..e28a723587db 100644 > --- a/drivers/gpu/drm/drm_client_modeset.c > +++ b/drivers/gpu/drm/drm_client_modeset.c > @@ -114,6 +114,20 @@ drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc) > return NULL; > } > > +static struct drm_display_mode * > +drm_connector_fallback_non_tiled_mode(struct drm_connector *connector) > +{ > + struct drm_display_mode *mode; > + > + list_for_each_entry(mode, &connector->modes, head) { > + if (mode->hdisplay == connector->tile_h_size && > + mode->vdisplay == connector->tile_v_size) > + continue; > + return mode; > + } > + return NULL; > +} > + > static struct drm_display_mode * > drm_connector_has_preferred_mode(struct drm_connector *connector, int width, int height) > { > @@ -348,8 +362,17 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, > struct drm_connector *connector; > u64 conn_configured = 0; > int tile_pass = 0; > + int num_tiled_conns = 0; > int i; > > + for (i = 0; i < connector_count; i++) { > + connector = connectors[i]; > + if (!connector->has_tile) > + continue; > + > + num_tiled_conns ++; Drop the space before the ++ here. Maybe just make this if (connectors[i]->has_tile) num_tiled_conns++; Reviewed-by: Dave Airlie <airlied@redhat.com> Otherwise I think this seems fine, though it does beg the question in my mind of what happens if I get 2 8K monitors, and plug the first tile of one in, and the second tile of the other in. Dave.
On Tue, 5 Nov 2019 at 07:00, Manasi Navare <manasi.d.navare@intel.com> wrote: > > On Mon, Nov 04, 2019 at 07:48:26PM +1000, David Airlie wrote: > > On Mon, Nov 4, 2019 at 7:18 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > > On Thu, Oct 31, 2019 at 02:48:39PM -0700, Manasi Navare wrote: > > > > In case of tiled displays, if we hotplug just one connector, > > > > fbcon currently just selects the preferred mode and if it is > > > > tiled mode then that becomes a problem if rest of the tiles are > > > > not present. > > > > So in the fbdev driver on hotplug when we probe the client modeset, > > > > we we dont find all the connectors for all tiles, then on a connector > > > > with one tile, just fallback to the first available non tiled mode > > > > to display over a single connector. > > > > > > > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Suggested-by: Dave Airlie <airlied@redhat.com> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Cc: Dave Airlie <airlied@redhat.com> > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > > Hm, should we mayb have a slight timeout first to wait for the 2nd > > > connector? Otherwise lots of flickering going on when plugging in one of > > > these screens ... > > > > Not really, > > > > There are 3 scenarios with the multi-cable tiled monitors. and > > non-resizeable fbdev. > > > > a) it's plugged in at boot. both cables are detected, fbdev gets a > > full tiled mode. > > Yes this works as expected > > > b) it's not plugged in at boot, the user starts plugging it in, fbdev > > was inited via the panel or another monitor. fbdev won't resize. > > c) it's half plugged in at boot, then you get a non-tiled mode, and > > fbdev can't resize to tiled anyways. > > In b and c, when its booted with only 1 cable connected and/or hotplugged only > one cable after boot, I fallback to fisrt available non tiled mode, does that sound good? > > > > > Also plugging in one of these multi-cable monitors piecemeal is going > > to take multiple seconds for the user to do physical cable plugging. > > So still good with ignoring the second hotplug completely? > However i donno where in the fb helper to stop passing the modeset down > after the drm_fb_helper_hotplug_event() I don't think you can ignore it completely. I think you just have to make the mode picking logic pick the right answer somehow. So you can tell if width/height are going to be lower than the tiled mode size, and in that case just don't enable the secondary tile. Dave.
Hi, On Wed, 6 Nov 2019 at 02:47, Dave Airlie <airlied@gmail.com> wrote: > Otherwise I think this seems fine, though it does beg the question in > my mind of what happens if I get 2 8K monitors, and plug the first > tile of one in, and the second tile of the other in. Honestly in that case I think 'you get to literally keep both pieces' is a reasonable answer. Cheers, Daniel
On Wed, Nov 06, 2019 at 12:47:16PM +1000, Dave Airlie wrote: > On Fri, 1 Nov 2019 at 07:46, Manasi Navare <manasi.d.navare@intel.com> wrote: > > > > In case of tiled displays, if we hotplug just one connector, > > fbcon currently just selects the preferred mode and if it is > > tiled mode then that becomes a problem if rest of the tiles are > > not present. > > So in the fbdev driver on hotplug when we probe the client modeset, > > we we dont find all the connectors for all tiles, then on a connector > > with one tile, just fallback to the first available non tiled mode > > to display over a single connector. > > > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Suggested-by: Dave Airlie <airlied@redhat.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Dave Airlie <airlied@redhat.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/drm_client_modeset.c | 29 ++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c > > index 895b73f23079..e28a723587db 100644 > > --- a/drivers/gpu/drm/drm_client_modeset.c > > +++ b/drivers/gpu/drm/drm_client_modeset.c > > @@ -114,6 +114,20 @@ drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc) > > return NULL; > > } > > > > +static struct drm_display_mode * > > +drm_connector_fallback_non_tiled_mode(struct drm_connector *connector) > > +{ > > + struct drm_display_mode *mode; > > + > > + list_for_each_entry(mode, &connector->modes, head) { > > + if (mode->hdisplay == connector->tile_h_size && > > + mode->vdisplay == connector->tile_v_size) > > + continue; > > + return mode; > > + } > > + return NULL; > > +} > > + > > static struct drm_display_mode * > > drm_connector_has_preferred_mode(struct drm_connector *connector, int width, int height) > > { > > @@ -348,8 +362,17 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, > > struct drm_connector *connector; > > u64 conn_configured = 0; > > int tile_pass = 0; > > + int num_tiled_conns = 0; > > int i; > > > > + for (i = 0; i < connector_count; i++) { > > + connector = connectors[i]; > > + if (!connector->has_tile) > > + continue; > > + > > + num_tiled_conns ++; > > Drop the space before the ++ here. Maybe just make this > > if (connectors[i]->has_tile) > num_tiled_conns++; Sure will modify like above and add your r-b afterwards. Thank you so much for your review. This only covers the hotplug case with 1 connector hotplugged and need to still modify the fb dev code to ignore the second hotplug which I cant seem to figure out how to avoid the second hotplug from going through a modeset and retain the first modeset on screen. Also I will send a follow up patch to fallback to first non tiled mode in case of connected boot. But its okay for that to be separate patch than this right? Regards Manasi > > Reviewed-by: Dave Airlie <airlied@redhat.com> > > Otherwise I think this seems fine, though it does beg the question in > my mind of what happens if I get 2 8K monitors, and plug the first > tile of one in, and the second tile of the other in. > > Dave.
On Wed, Nov 06, 2019 at 12:50:21PM +1000, Dave Airlie wrote: > On Tue, 5 Nov 2019 at 07:00, Manasi Navare <manasi.d.navare@intel.com> wrote: > > > > On Mon, Nov 04, 2019 at 07:48:26PM +1000, David Airlie wrote: > > > On Mon, Nov 4, 2019 at 7:18 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > > > > On Thu, Oct 31, 2019 at 02:48:39PM -0700, Manasi Navare wrote: > > > > > In case of tiled displays, if we hotplug just one connector, > > > > > fbcon currently just selects the preferred mode and if it is > > > > > tiled mode then that becomes a problem if rest of the tiles are > > > > > not present. > > > > > So in the fbdev driver on hotplug when we probe the client modeset, > > > > > we we dont find all the connectors for all tiles, then on a connector > > > > > with one tile, just fallback to the first available non tiled mode > > > > > to display over a single connector. > > > > > > > > > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > Suggested-by: Dave Airlie <airlied@redhat.com> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > Cc: Dave Airlie <airlied@redhat.com> > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > > > > Hm, should we mayb have a slight timeout first to wait for the 2nd > > > > connector? Otherwise lots of flickering going on when plugging in one of > > > > these screens ... > > > > > > Not really, > > > > > > There are 3 scenarios with the multi-cable tiled monitors. and > > > non-resizeable fbdev. > > > > > > a) it's plugged in at boot. both cables are detected, fbdev gets a > > > full tiled mode. > > > > Yes this works as expected > > > > > b) it's not plugged in at boot, the user starts plugging it in, fbdev > > > was inited via the panel or another monitor. fbdev won't resize. > > > c) it's half plugged in at boot, then you get a non-tiled mode, and > > > fbdev can't resize to tiled anyways. > > > > In b and c, when its booted with only 1 cable connected and/or hotplugged only > > one cable after boot, I fallback to fisrt available non tiled mode, does that sound good? > > > > > > > > Also plugging in one of these multi-cable monitors piecemeal is going > > > to take multiple seconds for the user to do physical cable plugging. > > > > So still good with ignoring the second hotplug completely? > > However i donno where in the fb helper to stop passing the modeset down > > after the drm_fb_helper_hotplug_event() > > I don't think you can ignore it completely. I think you just have to > make the mode picking logic pick the right answer somehow. > > So you can tell if width/height are going to be lower than the tiled > mode size, and in that case just don't enable the secondary tile. > Ithink this tiled monitor is behaving in a way where if there are two ports connected, on the reciever end it tries to join the outputs and if there is no output or a different output other than tiled mode then it cant display anything so the moment i connect the second port, it expects the full tiled output but since no modes on second connector, it doesnt display anytjing goes into power save It almost makes me rethink our logic of falling back to the non tiled mode for the first hotplug since atleast this monitor displays the single tile across the whole monitor by stretching it out and then after connecting second connector it can ideally go back to a full tiled display Thoughts? Manasi > Dave.
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index 895b73f23079..e28a723587db 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -114,6 +114,20 @@ drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc) return NULL; } +static struct drm_display_mode * +drm_connector_fallback_non_tiled_mode(struct drm_connector *connector) +{ + struct drm_display_mode *mode; + + list_for_each_entry(mode, &connector->modes, head) { + if (mode->hdisplay == connector->tile_h_size && + mode->vdisplay == connector->tile_v_size) + continue; + return mode; + } + return NULL; +} + static struct drm_display_mode * drm_connector_has_preferred_mode(struct drm_connector *connector, int width, int height) { @@ -348,8 +362,17 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, struct drm_connector *connector; u64 conn_configured = 0; int tile_pass = 0; + int num_tiled_conns = 0; int i; + for (i = 0; i < connector_count; i++) { + connector = connectors[i]; + if (!connector->has_tile) + continue; + + num_tiled_conns ++; + } + retry: for (i = 0; i < connector_count; i++) { connector = connectors[i]; @@ -394,6 +417,12 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, connector->base.id, connector->tile_group ? connector->tile_group->id : 0); modes[i] = drm_connector_has_preferred_mode(connector, width, height); } + if (connector->has_tile && + num_tiled_conns < connector->num_h_tile * connector->num_v_tile) { + DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n", + connector->base.id); + modes[i] = drm_connector_fallback_non_tiled_mode(connector); + } /* No preferred modes, pick one off the list */ if (!modes[i] && !list_empty(&connector->modes)) { list_for_each_entry(modes[i], &connector->modes, head)
In case of tiled displays, if we hotplug just one connector, fbcon currently just selects the preferred mode and if it is tiled mode then that becomes a problem if rest of the tiles are not present. So in the fbdev driver on hotplug when we probe the client modeset, we we dont find all the connectors for all tiles, then on a connector with one tile, just fallback to the first available non tiled mode to display over a single connector. Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Suggested-by: Dave Airlie <airlied@redhat.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Dave Airlie <airlied@redhat.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/drm_client_modeset.c | 29 ++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)