diff mbox series

[1/2] drm/nouveau/disp/nv50-: force scaler for any non-default LVDS/eDP modes

Message ID 20190525224149.4652-1-imirkin@alum.mit.edu (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/nouveau/disp/nv50-: force scaler for any non-default LVDS/eDP modes | expand

Commit Message

Ilia Mirkin May 25, 2019, 10:41 p.m. UTC
Higher layers tend to add a lot of modes not actually in the EDID, such
as the standard DMT modes. Changing this would be extremely intrusive to
everyone, so just force the scaler more often. There are no practical
cases we're aware of where a LVDS/eDP panel has multiple resolutions
exposed, and i915 already does it this way.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110660
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---

Untested for now, hoping that the bugzilla filer will test it out. Seems
obvious though.

 drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Ben Skeggs May 27, 2019, 6:24 a.m. UTC | #1
On Sun, 26 May 2019 at 08:41, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>
> Higher layers tend to add a lot of modes not actually in the EDID, such
> as the standard DMT modes. Changing this would be extremely intrusive to
> everyone, so just force the scaler more often. There are no practical
> cases we're aware of where a LVDS/eDP panel has multiple resolutions
> exposed, and i915 already does it this way.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110660
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Thanks Ilia, grabbed both patches.

> ---
>
> Untested for now, hoping that the bugzilla filer will test it out. Seems
> obvious though.
>
>  drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 134701a837c8..ef8d7a71564a 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -322,8 +322,13 @@ nv50_outp_atomic_check_view(struct drm_encoder *encoder,
>                 switch (connector->connector_type) {
>                 case DRM_MODE_CONNECTOR_LVDS:
>                 case DRM_MODE_CONNECTOR_eDP:
> -                       /* Force use of scaler for non-EDID modes. */
> -                       if (adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
> +                       /* Don't force scaler for EDID modes with
> +                        * same size as the native one (e.g. different
> +                        * refresh rate)
> +                        */
> +                       if (adjusted_mode->hdisplay == native_mode->hdisplay &&
> +                           adjusted_mode->vdisplay == native_mode->vdisplay &&
> +                           adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
>                                 break;
>                         mode = native_mode;
>                         asyc->scaler.full = true;
> --
> 2.21.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
Ilia Mirkin May 28, 2019, 8:36 p.m. UTC | #2
Sigh ... looks like this doesn't actually do what we want. See the
last couple comments in:

https://bugs.freedesktop.org/show_bug.cgi?id=110660

It seems to work as expected with "mode" instead of "adjusted_mode".
Does that make sense? It kinda does based on the later code, which
copies mode into adjusted_mode...

Assuming it makes sense to use "mode", Ben, want to just do a fixup
locally, or want me to send a revert + new patch?

  -ilia

On Mon, May 27, 2019 at 2:24 AM Ben Skeggs <skeggsb@gmail.com> wrote:
>
> On Sun, 26 May 2019 at 08:41, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> >
> > Higher layers tend to add a lot of modes not actually in the EDID, such
> > as the standard DMT modes. Changing this would be extremely intrusive to
> > everyone, so just force the scaler more often. There are no practical
> > cases we're aware of where a LVDS/eDP panel has multiple resolutions
> > exposed, and i915 already does it this way.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110660
> > Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> Thanks Ilia, grabbed both patches.
>
> > ---
> >
> > Untested for now, hoping that the bugzilla filer will test it out. Seems
> > obvious though.
> >
> >  drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > index 134701a837c8..ef8d7a71564a 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > @@ -322,8 +322,13 @@ nv50_outp_atomic_check_view(struct drm_encoder *encoder,
> >                 switch (connector->connector_type) {
> >                 case DRM_MODE_CONNECTOR_LVDS:
> >                 case DRM_MODE_CONNECTOR_eDP:
> > -                       /* Force use of scaler for non-EDID modes. */
> > -                       if (adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
> > +                       /* Don't force scaler for EDID modes with
> > +                        * same size as the native one (e.g. different
> > +                        * refresh rate)
> > +                        */
> > +                       if (adjusted_mode->hdisplay == native_mode->hdisplay &&
> > +                           adjusted_mode->vdisplay == native_mode->vdisplay &&
> > +                           adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
> >                                 break;
> >                         mode = native_mode;
> >                         asyc->scaler.full = true;
> > --
> > 2.21.0
> >
> > _______________________________________________
> > Nouveau mailing list
> > Nouveau@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
diff mbox series

Patch

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 134701a837c8..ef8d7a71564a 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -322,8 +322,13 @@  nv50_outp_atomic_check_view(struct drm_encoder *encoder,
 		switch (connector->connector_type) {
 		case DRM_MODE_CONNECTOR_LVDS:
 		case DRM_MODE_CONNECTOR_eDP:
-			/* Force use of scaler for non-EDID modes. */
-			if (adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
+			/* Don't force scaler for EDID modes with
+			 * same size as the native one (e.g. different
+			 * refresh rate)
+			 */
+			if (adjusted_mode->hdisplay == native_mode->hdisplay &&
+			    adjusted_mode->vdisplay == native_mode->vdisplay &&
+			    adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
 				break;
 			mode = native_mode;
 			asyc->scaler.full = true;