Re: [Intel-gfx] [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor

2017-02-20 Thread Daniel Vetter
For next time around, pls cc dri-devel (it's in MAINTAINERS,
get_maintainers.pl gets it right) too.
-Daniel

On Mon, Dec 5, 2016 at 10:49 PM, Pierre-Louis Bossart
 wrote:
> 100% reproducible issue found on SKL SkullCanyon NUC with two external
> DP daisy-chained monitors in DP/MST mode. When turning off or changing
> the input of the second monitor the machine stops with a kernel
> oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
>
> This issue is traced to an inconsistent control flow in
> drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
> the same time as 'req_payload.num_slots' is set to zero, but the pointer
> is dereferenced even when req_payload.num_slot is zero.
>
> The problematic dereference was introduced in commit dfda0df34
> ("drm/mst: rework payload table allocation to conform better")
> and may impact all versions since v3.18
>
> The fix suggested by Chris Wilson removes the kernel oops and was found to
> work well after 10mn of monkey-testing with the second monitor power and
> input buttons
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
> Cc: Dave Airlie 
> Signed-off-by: Pierre-Louis Bossart 
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index aa64448..f59771d 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct 
> drm_dp_mst_topology_mgr *mgr)
> mgr->payloads[i].vcpi = req_payload.vcpi;
> } else if (mgr->payloads[i].num_slots) {
> mgr->payloads[i].num_slots = 0;
> -   drm_dp_destroy_payload_step1(mgr, port, 
> port->vcpi.vcpi, >payloads[i]);
> +   drm_dp_destroy_payload_step1(mgr, port, 
> mgr->payloads[i].vcpi, >payloads[i]);
> req_payload.payload_state = 
> mgr->payloads[i].payload_state;
> mgr->payloads[i].start_slot = 0;
> }
> --
> 2.7.4
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor

2017-02-13 Thread Pandiyan, Dhinakaran
Looks like the right fix to me.

Reviewed-by: Dhinakaran Pandiyan 

On Wed, 2017-01-25 at 16:25 -0800, Nathan Ciobanu wrote:
> I tested this patch in dinq on a KBL system and it fixed the bug. The 
> system doesn't crash on disconnecting or powering off the second monitor 
> in the DP-MST chain. I also replied to the Bugzilla issue.
> 
> Tested-by: Nathan D Ciobanu 
> 
> On 12/05/2016 01:49 PM, Pierre-Louis Bossart wrote:
> > 100% reproducible issue found on SKL SkullCanyon NUC with two external
> > DP daisy-chained monitors in DP/MST mode. When turning off or changing
> > the input of the second monitor the machine stops with a kernel
> > oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
> >
> > This issue is traced to an inconsistent control flow in
> > drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
> > the same time as 'req_payload.num_slots' is set to zero, but the pointer
> > is dereferenced even when req_payload.num_slot is zero.
> >
> > The problematic dereference was introduced in commit dfda0df34
> > ("drm/mst: rework payload table allocation to conform better")
> > and may impact all versions since v3.18
> >
> > The fix suggested by Chris Wilson removes the kernel oops and was found to
> > work well after 10mn of monkey-testing with the second monitor power and
> > input buttons
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
> > Cc: Dave Airlie 
> > Signed-off-by: Pierre-Louis Bossart 
> > ---
> >   drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index aa64448..f59771d 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct 
> > drm_dp_mst_topology_mgr *mgr)
> > mgr->payloads[i].vcpi = req_payload.vcpi;
> > } else if (mgr->payloads[i].num_slots) {
> > mgr->payloads[i].num_slots = 0;
> > -   drm_dp_destroy_payload_step1(mgr, port, 
> > port->vcpi.vcpi, >payloads[i]);
> > +   drm_dp_destroy_payload_step1(mgr, port, 
> > mgr->payloads[i].vcpi, >payloads[i]);
> > req_payload.payload_state = 
> > mgr->payloads[i].payload_state;
> > mgr->payloads[i].start_slot = 0;
> > }
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor

2017-01-25 Thread Nathan Ciobanu
I tested this patch in dinq on a KBL system and it fixed the bug. The 
system doesn't crash on disconnecting or powering off the second monitor 
in the DP-MST chain. I also replied to the Bugzilla issue.


Tested-by: Nathan D Ciobanu 

On 12/05/2016 01:49 PM, Pierre-Louis Bossart wrote:

100% reproducible issue found on SKL SkullCanyon NUC with two external
DP daisy-chained monitors in DP/MST mode. When turning off or changing
the input of the second monitor the machine stops with a kernel
oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.

This issue is traced to an inconsistent control flow in
drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
the same time as 'req_payload.num_slots' is set to zero, but the pointer
is dereferenced even when req_payload.num_slot is zero.

The problematic dereference was introduced in commit dfda0df34
("drm/mst: rework payload table allocation to conform better")
and may impact all versions since v3.18

The fix suggested by Chris Wilson removes the kernel oops and was found to
work well after 10mn of monkey-testing with the second monitor power and
input buttons

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Cc: Dave Airlie 
Signed-off-by: Pierre-Louis Bossart 
---
  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index aa64448..f59771d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
mgr->payloads[i].vcpi = req_payload.vcpi;
} else if (mgr->payloads[i].num_slots) {
mgr->payloads[i].num_slots = 0;
-   drm_dp_destroy_payload_step1(mgr, port, 
port->vcpi.vcpi, >payloads[i]);
+   drm_dp_destroy_payload_step1(mgr, port, 
mgr->payloads[i].vcpi, >payloads[i]);
req_payload.payload_state = 
mgr->payloads[i].payload_state;
mgr->payloads[i].start_slot = 0;
}


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor

2017-01-25 Thread Nathan Ciobanu

I tested this on a KBL using 01-25-2017 dinq and it fixed the bug.


Tested-by: Nathan D Ciobanu 


On 12/05/2016 01:49 PM, Pierre-Louis Bossart wrote:

100% reproducible issue found on SKL SkullCanyon NUC with two external
DP daisy-chained monitors in DP/MST mode. When turning off or changing
the input of the second monitor the machine stops with a kernel
oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.

This issue is traced to an inconsistent control flow in
drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
the same time as 'req_payload.num_slots' is set to zero, but the pointer
is dereferenced even when req_payload.num_slot is zero.

The problematic dereference was introduced in commit dfda0df34
("drm/mst: rework payload table allocation to conform better")
and may impact all versions since v3.18

The fix suggested by Chris Wilson removes the kernel oops and was found to
work well after 10mn of monkey-testing with the second monitor power and
input buttons

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Cc: Dave Airlie 
Signed-off-by: Pierre-Louis Bossart 
---
  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index aa64448..f59771d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
mgr->payloads[i].vcpi = req_payload.vcpi;
} else if (mgr->payloads[i].num_slots) {
mgr->payloads[i].num_slots = 0;
-   drm_dp_destroy_payload_step1(mgr, port, 
port->vcpi.vcpi, >payloads[i]);
+   drm_dp_destroy_payload_step1(mgr, port, 
mgr->payloads[i].vcpi, >payloads[i]);
req_payload.payload_state = 
mgr->payloads[i].payload_state;
mgr->payloads[i].start_slot = 0;
}


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor

2016-12-05 Thread Pierre-Louis Bossart
100% reproducible issue found on SKL SkullCanyon NUC with two external
DP daisy-chained monitors in DP/MST mode. When turning off or changing
the input of the second monitor the machine stops with a kernel
oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.

This issue is traced to an inconsistent control flow in
drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
the same time as 'req_payload.num_slots' is set to zero, but the pointer
is dereferenced even when req_payload.num_slot is zero.

The problematic dereference was introduced in commit dfda0df34
("drm/mst: rework payload table allocation to conform better")
and may impact all versions since v3.18

The fix suggested by Chris Wilson removes the kernel oops and was found to
work well after 10mn of monkey-testing with the second monitor power and
input buttons

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Cc: Dave Airlie 
Signed-off-by: Pierre-Louis Bossart 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index aa64448..f59771d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
mgr->payloads[i].vcpi = req_payload.vcpi;
} else if (mgr->payloads[i].num_slots) {
mgr->payloads[i].num_slots = 0;
-   drm_dp_destroy_payload_step1(mgr, port, 
port->vcpi.vcpi, >payloads[i]);
+   drm_dp_destroy_payload_step1(mgr, port, 
mgr->payloads[i].vcpi, >payloads[i]);
req_payload.payload_state = 
mgr->payloads[i].payload_state;
mgr->payloads[i].start_slot = 0;
}
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx