| Message ID | 1418399858-26849-1-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 12, 2014 at 05:57:38PM +0200, Imre Deak wrote: > After > > commit a18c0af171bfb875012da26f23df051004726973 > uthor: Thierry Reding <treding@nvidia.com> > Date: Wed Dec 10 11:38:49 2014 +0100 > > drm: Zero out DRM object memory upon cleanup > > we will use the eDP encoder during destroying it. Fix this by calling > drm_encoder_cleanup() at a point when the encoder is not used any more. > This caused a NULL pointer dereference in pps_lock(), I can't see that > it caused any other problem. > > All the other encoders seem to call drm_encoder_cleanup() at a safe > place. > > Signed-off-by: Imre Deak <imre.deak@intel.com> drm_encoder_cleanup() doesn't appear to have any nasty side effects so this looks sane. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 3fc3296..0a55623 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4310,7 +4310,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) > > drm_dp_aux_unregister(&intel_dp->aux); > intel_dp_mst_encoder_cleanup(intel_dig_port); > - drm_encoder_cleanup(encoder); > if (is_edp(intel_dp)) { > cancel_delayed_work_sync(&intel_dp->panel_vdd_work); > /* > @@ -4326,6 +4325,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) > intel_dp->edp_notifier.notifier_call = NULL; > } > } > + drm_encoder_cleanup(encoder); > kfree(intel_dig_port); > } > > -- > 1.8.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 364/364 364/364
ILK +3-3 362/366 362/366
SNB 448/450 448/450
IVB -1 497/498 496/498
BYT 289/289 289/289
HSW 563/564 563/564
BDW 417/417 417/417
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
ILK igt_kms_flip_nonexisting-fb DMESG_WARN(1, M26)PASS(3, M37M26) PASS(1, M26)
ILK igt_kms_flip_rcs-flip-vs-panning-interruptible DMESG_WARN(1, M26)PASS(3, M37M26) PASS(1, M26)
ILK igt_kms_flip_rcs-wf_vblank-vs-dpms-interruptible DMESG_WARN(1, M26)PASS(2, M26M37) PASS(1, M26)
ILK igt_kms_flip_busy-flip-interruptible DMESG_WARN(1, M26)PASS(1, M26) DMESG_WARN(1, M26)
*ILK igt_kms_flip_rcs-flip-vs-panning PASS(2, M26) DMESG_WARN(1, M26)
*ILK igt_kms_flip_vblank-vs-hang PASS(2, M26) NSPT(1, M26)
*IVB igt_kms_cursor_crc_cursor-256x256-offscreen PASS(2, M4M21) DMESG_WARN(1, M21)
Note: You need to pay more attention to line start with '*'
On Fri, Dec 12, 2014 at 07:06:24PM +0200, Ville Syrjälä wrote: > On Fri, Dec 12, 2014 at 05:57:38PM +0200, Imre Deak wrote: > > After > > > > commit a18c0af171bfb875012da26f23df051004726973 > > uthor: Thierry Reding <treding@nvidia.com> > > Date: Wed Dec 10 11:38:49 2014 +0100 > > > > drm: Zero out DRM object memory upon cleanup Please cc Thierry next time around. I've added him. > > > > we will use the eDP encoder during destroying it. Fix this by calling > > drm_encoder_cleanup() at a point when the encoder is not used any more. > > This caused a NULL pointer dereference in pps_lock(), I can't see that > > it caused any other problem. > > > > All the other encoders seem to call drm_encoder_cleanup() at a safe > > place. > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > drm_encoder_cleanup() doesn't appear to have any nasty side effects > so this looks sane. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Queued for -next, thanks for the patch. -Daniel
On Mon, Dec 15, 2014 at 03:45:34PM +0100, Daniel Vetter wrote: > On Fri, Dec 12, 2014 at 07:06:24PM +0200, Ville Syrjälä wrote: > > On Fri, Dec 12, 2014 at 05:57:38PM +0200, Imre Deak wrote: > > > After > > > > > > commit a18c0af171bfb875012da26f23df051004726973 > > > uthor: Thierry Reding <treding@nvidia.com> > > > Date: Wed Dec 10 11:38:49 2014 +0100 > > > > > > drm: Zero out DRM object memory upon cleanup > > Please cc Thierry next time around. I've added him. I don't have a copy of the original patch in any of my inboxes. It'd be nice to Cc dri-devel@lists.freedesktop.org on patches of this nature. I had to go dig for the patch in the intel-gfx archives and it looks good to me, so if it's not too late yet: Reviewed-by: Thierry Reding <treding@nvidia.com>
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3fc3296..0a55623 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4310,7 +4310,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) drm_dp_aux_unregister(&intel_dp->aux); intel_dp_mst_encoder_cleanup(intel_dig_port); - drm_encoder_cleanup(encoder); if (is_edp(intel_dp)) { cancel_delayed_work_sync(&intel_dp->panel_vdd_work); /* @@ -4326,6 +4325,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) intel_dp->edp_notifier.notifier_call = NULL; } } + drm_encoder_cleanup(encoder); kfree(intel_dig_port); }
After commit a18c0af171bfb875012da26f23df051004726973 uthor: Thierry Reding <treding@nvidia.com> Date: Wed Dec 10 11:38:49 2014 +0100 drm: Zero out DRM object memory upon cleanup we will use the eDP encoder during destroying it. Fix this by calling drm_encoder_cleanup() at a point when the encoder is not used any more. This caused a NULL pointer dereference in pps_lock(), I can't see that it caused any other problem. All the other encoders seem to call drm_encoder_cleanup() at a safe place. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)