similar to: [PATCH AUTOSEL 4.19 070/167] drm/nouveau: Don't WARN_ON VCPI allocation failures

[ Upstream commit b513a18cf1d705bd04efd91c417e79e4938be093 ] This is much louder then we want. VCPI allocation failures are quite normal, since they will happen if any part of the modesetting process is interrupted by removing the DP MST topology in question. So just print a debugging message on VCPI failures instead. Signed-off-by: Lyude Paul <lyude at redhat.com> Fixes: f479c0ba4a17

Hi Greg, This feels like it's missing a From: line. commit b513a18cf1d705bd04efd91c417e79e4938be093 Author: Lyude Paul <lyude at redhat.com> Date: Mon Jan 28 16:03:50 2019 -0500 drm/nouveau: Don't WARN_ON VCPI allocation failures Is this an artifact of your notification-of-patches process and I never noticed before, or was the patch ingested incorrectly? Cheers, -ilia

This is much louder then we want. VCPI allocation failures are quite normal, since they will happen if any part of the modesetting process is interrupted by removing the DP MST topology in question. So just print a debugging message on VCPI failures instead. Signed-off-by: Lyude Paul <lyude at redhat.com> Fixes: f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2

Atomic checks should never modify anything outside of the state that they're passed in. Unfortunately this appears to be exactly what we're doing in nv50_msto_atomic_check() where we update mstc->pbn every time the function is called. This hasn't caused any bugs yet, but it needs to be fixed in order to ensure that when committing an artificially duplicated state (like during system

On Fri, Sep 13, 2019 at 09:33:36AM -0400, Ilia Mirkin wrote: >Hi Greg, > >This feels like it's missing a From: line. > >commit b513a18cf1d705bd04efd91c417e79e4938be093 >Author: Lyude Paul <lyude at redhat.com> >Date: Mon Jan 28 16:03:50 2019 -0500 > > drm/nouveau: Don't WARN_ON VCPI allocation failures > >Is this an artifact of your

On Fri, Sep 13, 2019 at 10:46:27AM -0400, Sasha Levin wrote: > On Fri, Sep 13, 2019 at 09:33:36AM -0400, Ilia Mirkin wrote: > > Hi Greg, > > > > This feels like it's missing a From: line. > > > > commit b513a18cf1d705bd04efd91c417e79e4938be093 > > Author: Lyude Paul <lyude at redhat.com> > > Date: Mon Jan 28 16:03:50 2019 -0500 > >

On Fri, Sep 13, 2019 at 11:01:11AM -0400, Sasha Levin wrote: > On Fri, Sep 13, 2019 at 03:54:56PM +0100, Greg Kroah-Hartman wrote: > > On Fri, Sep 13, 2019 at 10:46:27AM -0400, Sasha Levin wrote: > > > On Fri, Sep 13, 2019 at 09:33:36AM -0400, Ilia Mirkin wrote: > > > > Hi Greg, > > > > > > > > This feels like it's missing a From: line.

On Fri, Sep 13, 2019 at 11:01 AM Sasha Levin <sashal at kernel.org> wrote: > > On Fri, Sep 13, 2019 at 03:54:56PM +0100, Greg Kroah-Hartman wrote: > >On Fri, Sep 13, 2019 at 10:46:27AM -0400, Sasha Levin wrote: > >> On Fri, Sep 13, 2019 at 09:33:36AM -0400, Ilia Mirkin wrote: > >> > Hi Greg, > >> > > >> > This feels like it's

On Fri, Sep 13, 2019 at 03:54:56PM +0100, Greg Kroah-Hartman wrote: >On Fri, Sep 13, 2019 at 10:46:27AM -0400, Sasha Levin wrote: >> On Fri, Sep 13, 2019 at 09:33:36AM -0400, Ilia Mirkin wrote: >> > Hi Greg, >> > >> > This feels like it's missing a From: line. >> > >> > commit b513a18cf1d705bd04efd91c417e79e4938be093 >> > Author:

In order to be able to use bpc values that are different from what the connector reports, we want to be able to store the bpc value we decide on using for an atomic state in nv50_head_atom and refer to that instead of simply using the value that the connector reports throughout the whole atomic check phase and commit phase. This will let us (eventually) implement the max bpc connector property,

Currently, nouveau tries to go through the drm_dp_mst_port structures itself in order to retrieve the relevant payload and VCPI information that it needs to report to the GPU. This is wrong: mstc->port could be destroyed at any point, and additionally the payload could be changed at any point because it doesn't bother trying to grab the payload lock. So; remove nv50_msto_payload entirely

Currently, nouveau uses the yolo method of setting up MST displays: it uses the old VCPI helpers (drm_dp_find_vcpi_slots()) for computing the display configuration. These helpers don't take care to make sure they take a reference to the mstb port that they're checking, and additionally don't actually check whether or not the topology still has enough bandwidth to provide the VCPI

Currently, nouveau uses the yolo method of setting up MST displays: it uses the old VCPI helpers (drm_dp_find_vcpi_slots()) for computing the display configuration. These helpers don't take care to make sure they take a reference to the mstb port that they're checking, and additionally don't actually check whether or not the topology still has enough bandwidth to provide the VCPI

There is no need to look at the port's VCPI allocation before calling drm_dp_mst_deallocate_vcpi(), as we already have msto->disabled to let us avoid cleaning up an msto more then once. The DP MST core will never call drm_dp_mst_deallocate_vcpi() on it's own, which is presumably what these checks are meant to protect against. More importantly though, we're about to stop clearing

On 8/8/19 1:04 PM, Lyude Paul wrote: > I -thought- I had fixed this entirely, but it looks like that I didn't > test this thoroughly enough as we apparently still make one big mistake > with nv50_msto_atomic_check() - we don't handle the following scenario: > > * CRTC #1 has n VCPI allocated to it, is attached to connector DP-4 > which is attached to encoder #1.

It turns out that I forgot to go through and make sure that I converted all encoder callbacks to use atomic_enable/atomic_disable(), so let's go and actually do that. Signed-off-by: Lyude Paul <lyude at redhat.com> Cc: Kirill A. Shutemov <kirill at shutemov.name> Fixes: 09838c4efe9a ("drm/nouveau/kms: Search for encoders' connectors properly") ---

This fixes the extra issues I discovered upstream after the introduction of my rework of the atomic VCPI helpers that occur during suspend/resume. This time around, we use a slightly different but much less complicated approach for fixing said issues. Cc: Daniel Vetter <daniel at ffwll.ch> Lyude Paul (4): drm/dp_mst: Fix unbalanced malloc ref in drm_dp_mst_deallocate_vcpi()

Same as we did for i915, but for nouveau this time. Additionally, we grab a malloc reference to the port that lasts for the entire lifetime of nv50_mstc, which gives us the guarantee that mstc->port will always point to valid memory for as long as the mstc stays around. Signed-off-by: Lyude Paul <lyude at redhat.com> --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 18 +++++------------- 1

Looks like a regression got introduced into nv50_mstc_atomic_check() that somehow didn't get found until now. If userspace changes crtc_state->active to false but leaves the CRTC enabled, we end up calling drm_dp_atomic_find_vcpi_slots() using the PBN calculated in asyh->dp.pbn. However, if the display is inactive we end up calculating a PBN of 0, which inadvertently causes us to have an

I -thought- I had fixed this entirely, but it looks like that I didn't test this thoroughly enough as we apparently still make one big mistake with nv50_msto_atomic_check() - we don't handle the following scenario: * CRTC #1 has n VCPI allocated to it, is attached to connector DP-4 which is attached to encoder #1. enabled=y active=n * CRTC #1 is changed from DP-4 to DP-5, causing: *