[Nouveau] [Bug 108500] Crash when creating a depth buffer on GeForce 320M
https://bugs.freedesktop.org/show_bug.cgi?id=108500 --- Comment #11 from Ilia Mirkin --- Given the amount of time that this has gone on unfixed, I think we should just make mcp89 point at mcp77_mmu_new instead of g84_mmu_new (in nvkm/engine/device/base.c). Literally the only difference between those two is the ability to use compression. The quick test in comment #9 didn't yield positive results. Let's not make things extra-broken for people -- even if compression is somehow enableable on those chips, it's never worked on nouveau, I think. Timo - are you up to sending a change to fix the above in the kernel? If not, I can do it. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 108500] Crash when creating a depth buffer on GeForce 320M
https://bugs.freedesktop.org/show_bug.cgi?id=108500 --- Comment #10 from Timo Wiren --- Since Lubuntu 19.04 the crash has disappeared but I get broken depth testing instead in all GL applications, including glxgears. My workaround (disabling depth compression) still works. Current kernel: 5.0.0-13-generic Mesa: 19.0.2 -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 110572] Regularly System Crash: (ca. 1 hour) nouveau 0000:08:00.0: gr: PGRAPH TLB flush idle timeout fail and nouveau 0000:08:00.0: mmu: ce0 mmu invalidate timeout
https://bugs.freedesktop.org/show_bug.cgi?id=110572 --- Comment #14 from Linux Freak --- Created attachment 144284 --> https://bugs.freedesktop.org/attachment.cgi?id=144284=edit crash nouveau :08:00.0: timeout crashnouveau :08:00.0: timeout -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH v3 04/10] drm: Convert connector_helper_funcs->atomic_check to accept drm_atomic_state
On Thu, May 16, 2019 at 03:00:01PM +0300, Laurent Pinchart wrote:
> Hi Sean,
>
> On Mon, May 13, 2019 at 10:38:58AM -0400, Sean Paul wrote:
> > On Sat, May 11, 2019 at 3:12 PM Laurent Pinchart wrote:
> > > On Thu, May 02, 2019 at 03:49:46PM -0400, Sean Paul wrote:
> > >> From: Sean Paul
> > >>
> > >> Everyone who implements connector_helper_funcs->atomic_check reaches
> > >> into the connector state to get the atomic state. Instead of continuing
> > >> this pattern, change the callback signature to just give atomic state
> > >> and let the driver determine what it does and does not need from it.
> > >>
> > >> Eventually all atomic functions should do this, but that's just too much
> > >> busy work for me.
> > >
> > > Given that drivers also access the connector state, isn't this slightly
> > > more inefficient ?
> >
> > Inefficient in terms of what?
>
> In terms of the operation having to lookup the connector state, when the
> caller has it and can easily pass it. As Daniel commented, this may be
> the price to pay for a cleaner API, but I wonder how much overhead all
> the state tracking is costing.
>
It'd be interesting to quantify, but iirc the last time we benchmarked
atomic check commits they were virtually free (~thousands/s).
> > Agree that in isolation this patch might seem unnecessary, but it ties
> > in with the encoder and bridge CLs which accept drm_atomic_state in
>
> CLs ?
Googleism for patches, I usually catch these before sending... guess I missed
one.
Sean
>
> > their hooks. In general the idea is to convert all atomic functions to
> > take overall atomic state instead of just their object state. Reality
> > has proven to be more complicated and we need more access than what
> > the current implementation provides.
> >
> > Sean
> >
> > >> Changes in v3:
> > >> - Added to the set
> > >>
> > >> Cc: Daniel Vetter
> > >> Cc: Ville Syrjälä
> > >> Cc: Jani Nikula
> > >> Cc: Joonas Lahtinen
> > >> Cc: Rodrigo Vivi
> > >> Cc: Ben Skeggs
> > >> Cc: Laurent Pinchart
> > >> Cc: Kieran Bingham
> > >> Cc: Eric Anholt
> > >> Signed-off-by: Sean Paul
> > >> ---
> > >> drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
> > >> drivers/gpu/drm/i915/intel_atomic.c | 8 +---
> > >> drivers/gpu/drm/i915/intel_dp_mst.c | 7 ---
> > >> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> > >> drivers/gpu/drm/i915/intel_sdvo.c| 9 +
> > >> drivers/gpu/drm/i915/intel_tv.c | 8 +---
> > >> drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 +++--
> > >> drivers/gpu/drm/rcar-du/rcar_lvds.c | 12 +++-
> > >> drivers/gpu/drm/vc4/vc4_txp.c| 7 ---
> > >> include/drm/drm_modeset_helper_vtables.h | 2 +-
> > >> 10 files changed, 37 insertions(+), 27 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > >> b/drivers/gpu/drm/drm_atomic_helper.c
> > >> index 9d9e47276839..fa5a367507c1 100644
> > >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> > >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > >> @@ -683,7 +683,7 @@ drm_atomic_helper_check_modeset(struct drm_device
> > >> *dev,
> > >> }
> > >>
> > >> if (funcs->atomic_check)
> > >> - ret = funcs->atomic_check(connector,
> > >> new_connector_state);
> > >> + ret = funcs->atomic_check(connector, state);
> > >> if (ret)
> > >> return ret;
> > >>
> > >> @@ -725,7 +725,7 @@ drm_atomic_helper_check_modeset(struct drm_device
> > >> *dev,
> > >> continue;
> > >>
> > >> if (funcs->atomic_check)
> > >> - ret = funcs->atomic_check(connector,
> > >> new_connector_state);
> > >> + ret = funcs->atomic_check(connector, state);
> > >> if (ret)
> > >> return ret;
> > >> }
> > >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > >> b/drivers/gpu/drm/i915/intel_atomic.c
> > >> index b844e8840c6f..e8a5b82e9242 100644
> > >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> > >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > >> @@ -103,12 +103,14 @@ int
> > >> intel_digital_connector_atomic_set_property(struct drm_connector
> > >> *connector,
> > >> }
> > >>
> > >> int intel_digital_connector_atomic_check(struct drm_connector *conn,
> > >> - struct drm_connector_state
> > >> *new_state)
> > >> + struct drm_atomic_state *state)
> > >> {
> > >> + struct drm_connector_state *new_state =
> > >> + drm_atomic_get_new_connector_state(state, conn);
> > >> struct intel_digital_connector_state *new_conn_state =
> > >> to_intel_digital_connector_state(new_state);
> > >> struct drm_connector_state *old_state =
> > >> - drm_atomic_get_old_connector_state(new_state->state, conn);
> > >> +
Re: [Nouveau] [PATCH v3 04/10] drm: Convert connector_helper_funcs->atomic_check to accept drm_atomic_state
On Thu, May 16, 2019 at 02:07:34PM +0200, Daniel Vetter wrote: > On Thu, May 16, 2019 at 2:02 PM Laurent Pinchart > wrote: > > > > Hi Daniel, > > > > On Mon, May 13, 2019 at 04:47:47PM +0200, Daniel Vetter wrote: > > > On Sat, May 11, 2019 at 10:12:02PM +0300, Laurent Pinchart wrote: > > > > On Thu, May 02, 2019 at 03:49:46PM -0400, Sean Paul wrote: > > > >> From: Sean Paul > > > >> > > > >> Everyone who implements connector_helper_funcs->atomic_check reaches > > > >> into the connector state to get the atomic state. Instead of continuing > > > >> this pattern, change the callback signature to just give atomic state > > > >> and let the driver determine what it does and does not need from it. > > > >> > > > >> Eventually all atomic functions should do this, but that's just too > > > >> much > > > >> busy work for me. > > > > > > > > Given that drivers also access the connector state, isn't this slightly > > > > more inefficient ? > > > > > > It's atomic code, we're trying to optimize for clean code at the expense > > > of a bit of runtime overhead due to more pointer chasing. And I agree with > > > the general push, the pile of old/new_state pointers of various objects > > > we're passing around is confusing. Passing the overall drm_atomic_state > > > seems much more reasonable, and with that we can get everything else. Plus > > > it's much more obvious whether you have the old/new state (since that's > > > explicit when you look it up from the drm_atomic_state). > > > > Yes, I agree it's cleaner. I just hope the atomic state tracking cost > > can be kept under control :-) > > > > By the way, this is likely not going to happen as it would be way too > > intrusive, but it would be nice to rename drm_atomic_state to > > drm_atomic_transaction (or something similar). It doesn't model a state, > > but a change between an old state to a new state. This confused me in > > the past, and I'm sure it can still be confusing to newcomers. > > Why are you the first to suggest this, this is awesome! Can't quite tell if that's irony or not. Anyways, this has been suggested before but no volunteers stepped forward. > drm_atomic_state is indeed not a state, but a transaction representing > how we go from the old to the new state. On a semi-related topic, I've occasionally pondered about moving mode_changed & co. from the obj states to the top level state/transaction (maybe stored as a bitmask). But that would definitely not be a trivial sed job. -- Ville Syrjälä Intel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH v3 04/10] drm: Convert connector_helper_funcs->atomic_check to accept drm_atomic_state
On Thu, May 16, 2019 at 2:02 PM Laurent Pinchart wrote: > > Hi Daniel, > > On Mon, May 13, 2019 at 04:47:47PM +0200, Daniel Vetter wrote: > > On Sat, May 11, 2019 at 10:12:02PM +0300, Laurent Pinchart wrote: > > > On Thu, May 02, 2019 at 03:49:46PM -0400, Sean Paul wrote: > > >> From: Sean Paul > > >> > > >> Everyone who implements connector_helper_funcs->atomic_check reaches > > >> into the connector state to get the atomic state. Instead of continuing > > >> this pattern, change the callback signature to just give atomic state > > >> and let the driver determine what it does and does not need from it. > > >> > > >> Eventually all atomic functions should do this, but that's just too much > > >> busy work for me. > > > > > > Given that drivers also access the connector state, isn't this slightly > > > more inefficient ? > > > > It's atomic code, we're trying to optimize for clean code at the expense > > of a bit of runtime overhead due to more pointer chasing. And I agree with > > the general push, the pile of old/new_state pointers of various objects > > we're passing around is confusing. Passing the overall drm_atomic_state > > seems much more reasonable, and with that we can get everything else. Plus > > it's much more obvious whether you have the old/new state (since that's > > explicit when you look it up from the drm_atomic_state). > > Yes, I agree it's cleaner. I just hope the atomic state tracking cost > can be kept under control :-) > > By the way, this is likely not going to happen as it would be way too > intrusive, but it would be nice to rename drm_atomic_state to > drm_atomic_transaction (or something similar). It doesn't model a state, > but a change between an old state to a new state. This confused me in > the past, and I'm sure it can still be confusing to newcomers. Why are you the first to suggest this, this is awesome! drm_atomic_state is indeed not a state, but a transaction representing how we go from the old to the new state. I think this is awesome enough that we should actually try to make it happen. Tree-wide cocci + bribing Dave on St. Patrick's day with lots of beer might be enough :-) -Daniel > > If we ever see this show up in profile, and it starts mattering, first > > thing we need is a hashtable I think (atm it's list walking, which is just > > terrible). But thus far no one cares. > > > > >> Changes in v3: > > >> - Added to the set > > >> > > >> Cc: Daniel Vetter > > >> Cc: Ville Syrjälä > > >> Cc: Jani Nikula > > >> Cc: Joonas Lahtinen > > >> Cc: Rodrigo Vivi > > >> Cc: Ben Skeggs > > >> Cc: Laurent Pinchart > > >> Cc: Kieran Bingham > > >> Cc: Eric Anholt > > >> Signed-off-by: Sean Paul > > >> --- > > >> drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- > > >> drivers/gpu/drm/i915/intel_atomic.c | 8 +--- > > >> drivers/gpu/drm/i915/intel_dp_mst.c | 7 --- > > >> drivers/gpu/drm/i915/intel_drv.h | 2 +- > > >> drivers/gpu/drm/i915/intel_sdvo.c| 9 + > > >> drivers/gpu/drm/i915/intel_tv.c | 8 +--- > > >> drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 +++-- > > >> drivers/gpu/drm/rcar-du/rcar_lvds.c | 12 +++- > > >> drivers/gpu/drm/vc4/vc4_txp.c| 7 --- > > >> include/drm/drm_modeset_helper_vtables.h | 2 +- > > >> 10 files changed, 37 insertions(+), 27 deletions(-) > > >> > > >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > >> b/drivers/gpu/drm/drm_atomic_helper.c > > >> index 9d9e47276839..fa5a367507c1 100644 > > >> --- a/drivers/gpu/drm/drm_atomic_helper.c > > >> +++ b/drivers/gpu/drm/drm_atomic_helper.c > > >> @@ -683,7 +683,7 @@ drm_atomic_helper_check_modeset(struct drm_device > > >> *dev, > > >>} > > >> > > >>if (funcs->atomic_check) > > >> - ret = funcs->atomic_check(connector, > > >> new_connector_state); > > >> + ret = funcs->atomic_check(connector, state); > > >>if (ret) > > >>return ret; > > >> > > >> @@ -725,7 +725,7 @@ drm_atomic_helper_check_modeset(struct drm_device > > >> *dev, > > >>continue; > > >> > > >>if (funcs->atomic_check) > > >> - ret = funcs->atomic_check(connector, > > >> new_connector_state); > > >> + ret = funcs->atomic_check(connector, state); > > >>if (ret) > > >>return ret; > > >>} > > >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c > > >> b/drivers/gpu/drm/i915/intel_atomic.c > > >> index b844e8840c6f..e8a5b82e9242 100644 > > >> --- a/drivers/gpu/drm/i915/intel_atomic.c > > >> +++ b/drivers/gpu/drm/i915/intel_atomic.c > > >> @@ -103,12 +103,14 @@ int > > >> intel_digital_connector_atomic_set_property(struct drm_connector > > >> *connector, > > >> } > > >> > > >> int intel_digital_connector_atomic_check(struct drm_connector *conn, > > >> - struct drm_connector_state > > >> *new_state) >
Re: [PATCH v3 04/10] drm: Convert connector_helper_funcs->atomic_check to accept drm_atomic_state
Hi Daniel,
On Mon, May 13, 2019 at 04:47:47PM +0200, Daniel Vetter wrote:
> On Sat, May 11, 2019 at 10:12:02PM +0300, Laurent Pinchart wrote:
> > On Thu, May 02, 2019 at 03:49:46PM -0400, Sean Paul wrote:
> >> From: Sean Paul
> >>
> >> Everyone who implements connector_helper_funcs->atomic_check reaches
> >> into the connector state to get the atomic state. Instead of continuing
> >> this pattern, change the callback signature to just give atomic state
> >> and let the driver determine what it does and does not need from it.
> >>
> >> Eventually all atomic functions should do this, but that's just too much
> >> busy work for me.
> >
> > Given that drivers also access the connector state, isn't this slightly
> > more inefficient ?
>
> It's atomic code, we're trying to optimize for clean code at the expense
> of a bit of runtime overhead due to more pointer chasing. And I agree with
> the general push, the pile of old/new_state pointers of various objects
> we're passing around is confusing. Passing the overall drm_atomic_state
> seems much more reasonable, and with that we can get everything else. Plus
> it's much more obvious whether you have the old/new state (since that's
> explicit when you look it up from the drm_atomic_state).
Yes, I agree it's cleaner. I just hope the atomic state tracking cost
can be kept under control :-)
By the way, this is likely not going to happen as it would be way too
intrusive, but it would be nice to rename drm_atomic_state to
drm_atomic_transaction (or something similar). It doesn't model a state,
but a change between an old state to a new state. This confused me in
the past, and I'm sure it can still be confusing to newcomers.
> If we ever see this show up in profile, and it starts mattering, first
> thing we need is a hashtable I think (atm it's list walking, which is just
> terrible). But thus far no one cares.
>
> >> Changes in v3:
> >> - Added to the set
> >>
> >> Cc: Daniel Vetter
> >> Cc: Ville Syrjälä
> >> Cc: Jani Nikula
> >> Cc: Joonas Lahtinen
> >> Cc: Rodrigo Vivi
> >> Cc: Ben Skeggs
> >> Cc: Laurent Pinchart
> >> Cc: Kieran Bingham
> >> Cc: Eric Anholt
> >> Signed-off-by: Sean Paul
> >> ---
> >> drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
> >> drivers/gpu/drm/i915/intel_atomic.c | 8 +---
> >> drivers/gpu/drm/i915/intel_dp_mst.c | 7 ---
> >> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> >> drivers/gpu/drm/i915/intel_sdvo.c| 9 +
> >> drivers/gpu/drm/i915/intel_tv.c | 8 +---
> >> drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 +++--
> >> drivers/gpu/drm/rcar-du/rcar_lvds.c | 12 +++-
> >> drivers/gpu/drm/vc4/vc4_txp.c| 7 ---
> >> include/drm/drm_modeset_helper_vtables.h | 2 +-
> >> 10 files changed, 37 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> >> b/drivers/gpu/drm/drm_atomic_helper.c
> >> index 9d9e47276839..fa5a367507c1 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -683,7 +683,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> >>}
> >>
> >>if (funcs->atomic_check)
> >> - ret = funcs->atomic_check(connector,
> >> new_connector_state);
> >> + ret = funcs->atomic_check(connector, state);
> >>if (ret)
> >>return ret;
> >>
> >> @@ -725,7 +725,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> >>continue;
> >>
> >>if (funcs->atomic_check)
> >> - ret = funcs->atomic_check(connector,
> >> new_connector_state);
> >> + ret = funcs->atomic_check(connector, state);
> >>if (ret)
> >>return ret;
> >>}
> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> >> b/drivers/gpu/drm/i915/intel_atomic.c
> >> index b844e8840c6f..e8a5b82e9242 100644
> >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >> @@ -103,12 +103,14 @@ int
> >> intel_digital_connector_atomic_set_property(struct drm_connector
> >> *connector,
> >> }
> >>
> >> int intel_digital_connector_atomic_check(struct drm_connector *conn,
> >> - struct drm_connector_state *new_state)
> >> + struct drm_atomic_state *state)
> >> {
> >> + struct drm_connector_state *new_state =
> >> + drm_atomic_get_new_connector_state(state, conn);
> >>struct intel_digital_connector_state *new_conn_state =
> >>to_intel_digital_connector_state(new_state);
> >>struct drm_connector_state *old_state =
> >> - drm_atomic_get_old_connector_state(new_state->state, conn);
> >> + drm_atomic_get_old_connector_state(state, conn);
> >>struct intel_digital_connector_state *old_conn_state =
> >>
Re: [PATCH v3 04/10] drm: Convert connector_helper_funcs->atomic_check to accept drm_atomic_state
Hi Sean,
On Mon, May 13, 2019 at 10:38:58AM -0400, Sean Paul wrote:
> On Sat, May 11, 2019 at 3:12 PM Laurent Pinchart wrote:
> > On Thu, May 02, 2019 at 03:49:46PM -0400, Sean Paul wrote:
> >> From: Sean Paul
> >>
> >> Everyone who implements connector_helper_funcs->atomic_check reaches
> >> into the connector state to get the atomic state. Instead of continuing
> >> this pattern, change the callback signature to just give atomic state
> >> and let the driver determine what it does and does not need from it.
> >>
> >> Eventually all atomic functions should do this, but that's just too much
> >> busy work for me.
> >
> > Given that drivers also access the connector state, isn't this slightly
> > more inefficient ?
>
> Inefficient in terms of what?
In terms of the operation having to lookup the connector state, when the
caller has it and can easily pass it. As Daniel commented, this may be
the price to pay for a cleaner API, but I wonder how much overhead all
the state tracking is costing.
> Agree that in isolation this patch might seem unnecessary, but it ties
> in with the encoder and bridge CLs which accept drm_atomic_state in
CLs ?
> their hooks. In general the idea is to convert all atomic functions to
> take overall atomic state instead of just their object state. Reality
> has proven to be more complicated and we need more access than what
> the current implementation provides.
>
> Sean
>
> >> Changes in v3:
> >> - Added to the set
> >>
> >> Cc: Daniel Vetter
> >> Cc: Ville Syrjälä
> >> Cc: Jani Nikula
> >> Cc: Joonas Lahtinen
> >> Cc: Rodrigo Vivi
> >> Cc: Ben Skeggs
> >> Cc: Laurent Pinchart
> >> Cc: Kieran Bingham
> >> Cc: Eric Anholt
> >> Signed-off-by: Sean Paul
> >> ---
> >> drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
> >> drivers/gpu/drm/i915/intel_atomic.c | 8 +---
> >> drivers/gpu/drm/i915/intel_dp_mst.c | 7 ---
> >> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> >> drivers/gpu/drm/i915/intel_sdvo.c| 9 +
> >> drivers/gpu/drm/i915/intel_tv.c | 8 +---
> >> drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 +++--
> >> drivers/gpu/drm/rcar-du/rcar_lvds.c | 12 +++-
> >> drivers/gpu/drm/vc4/vc4_txp.c| 7 ---
> >> include/drm/drm_modeset_helper_vtables.h | 2 +-
> >> 10 files changed, 37 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> >> b/drivers/gpu/drm/drm_atomic_helper.c
> >> index 9d9e47276839..fa5a367507c1 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -683,7 +683,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> >> }
> >>
> >> if (funcs->atomic_check)
> >> - ret = funcs->atomic_check(connector,
> >> new_connector_state);
> >> + ret = funcs->atomic_check(connector, state);
> >> if (ret)
> >> return ret;
> >>
> >> @@ -725,7 +725,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> >> continue;
> >>
> >> if (funcs->atomic_check)
> >> - ret = funcs->atomic_check(connector,
> >> new_connector_state);
> >> + ret = funcs->atomic_check(connector, state);
> >> if (ret)
> >> return ret;
> >> }
> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> >> b/drivers/gpu/drm/i915/intel_atomic.c
> >> index b844e8840c6f..e8a5b82e9242 100644
> >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >> @@ -103,12 +103,14 @@ int
> >> intel_digital_connector_atomic_set_property(struct drm_connector
> >> *connector,
> >> }
> >>
> >> int intel_digital_connector_atomic_check(struct drm_connector *conn,
> >> - struct drm_connector_state
> >> *new_state)
> >> + struct drm_atomic_state *state)
> >> {
> >> + struct drm_connector_state *new_state =
> >> + drm_atomic_get_new_connector_state(state, conn);
> >> struct intel_digital_connector_state *new_conn_state =
> >> to_intel_digital_connector_state(new_state);
> >> struct drm_connector_state *old_state =
> >> - drm_atomic_get_old_connector_state(new_state->state, conn);
> >> + drm_atomic_get_old_connector_state(state, conn);
> >> struct intel_digital_connector_state *old_conn_state =
> >> to_intel_digital_connector_state(old_state);
> >> struct drm_crtc_state *crtc_state;
> >> @@ -118,7 +120,7 @@ int intel_digital_connector_atomic_check(struct
> >> drm_connector *conn,
> >> if (!new_state->crtc)
> >> return 0;
> >>
> >> - crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> >> new_state->crtc);
> >> + crtc_state = drm_atomic_get_new_crtc_state(state,
Re: [Nouveau] [PATCH] drm/nouveau: fix duplication of nv50_head_atom struct
On Wed, May 15, 2019 at 11:29:40PM -0400, Ilia Mirkin wrote:
> On Tue, May 14, 2019 at 3:57 PM Peteris Rudzusiks
> wrote:
> >
> > On Tue, May 14, 2019 at 04:55:05PM +1000, Ben Skeggs wrote:
> > > On Sun, 12 May 2019 at 04:23, Peteris Rudzusiks
> > > wrote:
> > > >
> > > > nv50_head_atomic_duplicate_state() makes a copy of nv50_head_atom
> > > > struct. This patch adds copying of struct member named "or", which
> > > > previously was left uninitialized in the duplicated structure.
> > > >
> > > > Due to this bug, incorrect nhsync and nvsync values were sometimes used.
> > > > In my particular case, that lead to a mismatch between the output
> > > > resolution of the graphics device (GeForce GT 630 OEM) and the reported
> > > > input signal resolution on the display. xrandr reported 1680x1050, but
> > > > the display reported 1280x1024. As a result of this mismatch, the output
> > > > on the display looked like it was cropped (only part of the output was
> > > > actually visible on the display).
> > > >
> > > > git bisect pointed to commit 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle
> > > > SetControlOutputResource from head"), which added the member "or" to
> > > > nv50_head_atom structure, but forgot to copy it in
> > > > nv50_head_atomic_duplicate_state().
> > > >
> > > > Fixes: 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle
> > > > SetControlOutputResource from head")
> > > > Signed-off-by: Peteris Rudzusiks
> > > Oops, nice catch. Thank you for this, I've merged it in my tree and
> > > will get it upstream ASAP.
> > >
> > > Thanks,
> > > Ben.
> > >
> > Hi Ben,
> >
> > Thank you for taking the time to review and merge this patch.
> >
> > I'm new to the Linux kernel development process, so I am not sure what
> > happens next. Does inclusion in your tree imply that this fix will end
> > up in some (most likely - next) mainline kernel? Will it also be
> > backported to 4.19 LTS branch?
> >
> > This bug affects all kernel versions starting from v4.18. Probably not
> > that many systems though.
>
> Ben submits a pull request to Dave Airlie (drm maintainer), and Dave
> submits one to Linus for inclusion in the "official" upstream
> repository. Dave just sent a pull request to Linus, who usually picks
> these up within a few days (exceptions apply).
>
> Once in the mainline tree, the "Fixes" tag is likely to cause it to
> get picked up for stable. You can also nominate it for stable kernel
> branch inclusion explicitly (there are instructions somewhere, but
> basically you send an email to some list saying "please include commit
> ABC in kernels XYZ").
>
> What Ubuntu ships is, ultimately, up to Ubuntu. They will, however,
> frequently follow the stable kernel branches, and listen to the list
> above as well.
>
> Hope this helps,
>
> -ilia
Thanks for explaing this. I'll wait and see if this patch gets included
in stable releases without explicitly asking for it.
Regards,
Peteris
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
