similar to: [PATCH] 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

From: Lyude Paul <lyude at redhat.com> [ 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

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

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

From: Colin Ian King <colin.king at canonical.com> The null check on the array msto is incorrect since msto is never null. The null check should be instead on msto[i] since this is being dereferenced in the call to drm_mode_connector_attach_encoder. Thanks to Emil Velikov for pointing out the mistake in my original fix and for suggesting the correct fix. Detected by CoverityScan,

On 17/08/17 23:07, Ilia Mirkin wrote: > On Thu, Aug 17, 2017 at 6:03 PM, Colin King <colin.king at canonical.com> wrote: >> From: Colin Ian King <colin.king at canonical.com> >> >> The null check on the array msto is incorrect since msto is never >> null. The null check should be instead on msto[i] since this is >> being dereferenced in the call to

We need to actually make sure we check this on resume since otherwise we won't know whether or not the topology is still there once we've resumed, which will cause us to still think the topology is connected even after it's been removed if the removal happens mid-suspend. Signed-off-by: Lyude Paul <lyude at redhat.com> Cc: stable at vger.kernel.org ---

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,

Since we now have an easy way of refcounting drm_dp_mst_port structs and safely accessing their contents, there isn't any good reason to keep validating ports here. It doesn't prevent us from performing modesets on branch devices that have been removed either, and we already disallow enabling new displays on unregistered connectors in update_connector_routing() in

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.

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 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

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