search for: dp_interlac

On Wed, 26 Aug 2020 at 02:52, Lyude Paul <lyude at redhat.com> wrote: > > On Tue, 2020-08-25 at 08:28 +1000, Ben Skeggs wrote: > > On Tue, 25 Aug 2020 at 04:33, Lyude Paul <lyude at redhat.com> wrote: > > > Not entirely sure why this never came up when I originally tested this > > > (maybe some BIOSes already have this setup?) but the ->caps_init vfunc

Currently, nouveau doesn't actually bother to try probing whether or not it can actually handle interlaced modes over DisplayPort. As a result, on volta and later we'll end up trying to set an interlaced mode even when it's not supported and cause the front end for the display engine to hang. So, let's teach nouveau to reject interlaced modes on hardware that can't actually

...{ - const unsigned min_clock = 25000; - unsigned max_clock, ds_clock, clock; + const unsigned int min_clock = 25000; + unsigned int max_clock, ds_clock, clock; + const u8 bpp = 18; /* 6 bpc */ enum drm_mode_status ret; if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace) return MODE_NO_INTERLACE; max_clock = outp->dp.link_nr * outp->dp.link_bw; - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, - outp->dp.downstream_ports); - if (ds_clock) - max_clock = min(max_clock, ds_clock); - - clock = mode->clock * (connector->displa...

...return get_slave_funcs(encoder)->mode_valid(encoder, mode); > case DCB_OUTPUT_DP: > if (mode->flags & DRM_MODE_FLAG_INTERLACE && > - !nv_encoder->dp.caps.interlace) > + !nv_encoder->caps.dp_interlace) > return MODE_NO_INTERLACE; > > max_clock = nv_encoder->dp.link_nr; You probably meant for this hunk to go into an earlier patch. -ilia

...Check with the minmum bpc always, so we can advertise better modes. + * In particlar not doing this causes modes to be dropped on HDR + * displays as we might check with a bpc of 16 even. + */ + const u8 bpp = 6 * 3; if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace) return MODE_NO_INTERLACE; -- 2.39.2

Currently, nouveau doesn't actually bother to try probing whether or not it can actually handle interlaced modes over DisplayPort. As a result, on volta and later we'll end up trying to set an interlaced mode even when it's not supported and cause the front end for the display engine to hang. So, let's teach nouveau to reject interlaced modes on hardware that can't actually

...wer the bpc. So Ilia's concerns seem well founded. > Without that logic I guess you should just use > connector->display_info.bpc here as well. > > > enum drm_mode_status ret; > > > > if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace) > > return MODE_NO_INTERLACE; > > > > max_clock = outp->dp.link_nr * outp->dp.link_bw; > > - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, > > - outp->dp.downstream_ports); > > - if (ds_clock) > > - max_clock = min(ma...

Not entirely sure why this never came up when I originally tested this (maybe some BIOSes already have this setup?) but the ->caps_init vfunc appears to cause the display engine to throw an exception on driver init, at least on my ThinkPad P72: nouveau 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c 0000102b This is magic nvidia speak for "You need to have the DMA notifier

...truct drm_connector *connector, unsigned *out_clock) { const unsigned min_clock = 25000; - unsigned max_clock, ds_clock, clock; - enum drm_mode_status ret; + unsigned max_clock, ds_clock, clock = mode->clock; if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace) return MODE_NO_INTERLACE; + if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == DRM_MODE_FLAG_3D_FRAME_PACKING) + clock *= 2; + max_clock = outp->dp.link_nr * outp->dp.link_bw; ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, outp->dp.downstream_ports); @@ -...

Just a backport of the two patches for v5.9 that you'll want to apply. The first one was Cc'd to stable, but I forgot to Cc the second one as well. Lyude Paul (2): drm/nouveau/kms/nv50-: Get rid of bogus nouveau_conn_mode_valid() drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid() drivers/gpu/drm/nouveau/nouveau_connector.c | 36 ++++++---------------

..., ds_clock, clock; > + const unsigned int min_clock = 25000; > + unsigned int max_clock, ds_clock, clock; > + const u8 bpp = 18; /* 6 bpc */ > enum drm_mode_status ret; > > if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace) > return MODE_NO_INTERLACE; > > max_clock = outp->dp.link_nr * outp->dp.link_bw; > - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, > - outp->dp.downstream_ports); > - if (d...

...thout any fallback logic to lower the bpc. So Ilia's concerns seem well founded. Without that logic I guess you should just use connector->display_info.bpc here as well. > enum drm_mode_status ret; > > if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace) > return MODE_NO_INTERLACE; > > max_clock = outp->dp.link_nr * outp->dp.link_bw; > - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, > - outp->dp.downstream_ports); > - if (ds_clock) > - max_clock = min(max_clock, ds_clock); > - > - c...

...,19 @@ nv50_dp_mode_valid(struct drm_connector *connector, unsigned *out_clock) { const unsigned min_clock = 25000; - unsigned max_clock, clock; + unsigned max_clock, ds_clock, clock; enum drm_mode_status ret; if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace) return MODE_NO_INTERLACE; max_clock = outp->dp.link_nr * outp->dp.link_bw; - clock = mode->clock * (connector->display_info.bpc * 3) / 10; + ds_clock = drm_dp_downstream_max_clock(outp->dp.dpcd, + outp->dp.downstream_ports); + if (ds_clock) + max_clock = min(m...

...065,7 +1065,7 @@ nouveau_connector_mode_valid(struct drm_connector *connector, return get_slave_funcs(encoder)->mode_valid(encoder, mode); case DCB_OUTPUT_DP: if (mode->flags & DRM_MODE_FLAG_INTERLACE && - !nv_encoder->dp.caps.interlace) + !nv_encoder->caps.dp_interlace) return MODE_NO_INTERLACE; max_clock = nv_encoder->dp.link_nr; -- 2.26.2

...cerns seem well founded. > > Without that logic I guess you should just use > > connector->display_info.bpc here as well. > > > > > enum drm_mode_status ret; > > > > > > if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace) > > > return MODE_NO_INTERLACE; > > > > > > max_clock = outp->dp.link_nr * outp->dp.link_bw; > > > - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, > > > - outp->dp.downstream_ports); > > > - if (ds_clock) &...

These didn't apply cleanly to v5.8, so here's a backported version. Lyude Paul (2): drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps drm/nouveau/kms/nv50-: Log SOR/PIOR caps drivers/gpu/drm/nouveau/dispnv50/core507d.c | 25 ++++++++++++++++----- drivers/gpu/drm/nouveau/dispnv50/disp.c | 10 +++++++++ 2 files changed, 29 insertions(+), 6 deletions(-)

Not entirely sure why this never came up when I originally tested this (maybe some BIOSes already have this setup?) but the ->caps_init vfunc appears to cause the display engine to throw an exception on driver init, at least on my ThinkPad P72: nouveau 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c 0000102b This is magic nvidia speak for "You need to have the DMA notifier

...unsigned int min_clock = 25000; > > + unsigned int max_clock, ds_clock, clock; > > + const u8 bpp = 18; /* 6 bpc */ > > enum drm_mode_status ret; > > > > if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp- > > >caps.dp_interlace) > > return MODE_NO_INTERLACE; > > > > max_clock = outp->dp.link_nr * outp->dp.link_bw; > > - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd, > > - outp- > > >dp....

Most of the reason I'm asking for an RFC here is because this code pulls a lot of code out of i915 and into shared DP helpers. Anyway-nouveau's HPD related code has been collecting dust for a while. Other then the occasional runtime PM related and MST related fixes, we're missing a lot of nice things that have been added to DRM since this was originally written. Additionally, the code

Most of the reason I'm asking for an RFC here is because this code pulls a lot of code out of i915 and into shared DP helpers. Anyway-nouveau's HPD related code has been collecting dust for a while. Other then the occasional runtime PM related and MST related fixes, we're missing a lot of nice things that have been added to DRM since this was originally written. Additionally, the code