* [PATCH] drm/i915: reset drm state backpointer in crtc_state @ 2015-04-06 18:30 Chandra Konduru 2015-04-06 20:52 ` shuang.he ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Chandra Konduru @ 2015-04-06 18:30 UTC (permalink / raw) To: intel-gfx At end of intel_crtc_set_config, reset crtc_state's drm_state back pointer to null. Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c84926b..f9c2e4d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12451,8 +12451,10 @@ fail: } out_config: - if (state) + if (state) { drm_atomic_state_free(state); + to_intel_crtc(set->crtc)->config->base.state = NULL; + } intel_set_config_free(config); return ret; -- 1.7.9.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: reset drm state backpointer in crtc_state 2015-04-06 18:30 [PATCH] drm/i915: reset drm state backpointer in crtc_state Chandra Konduru @ 2015-04-06 20:52 ` shuang.he 2015-04-07 9:02 ` Jani Nikula 2015-04-08 8:22 ` Daniel Vetter 2 siblings, 0 replies; 7+ messages in thread From: shuang.he @ 2015-04-06 20:52 UTC (permalink / raw) To: shuang.he, ethan.gao, intel-gfx, chandra.konduru Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) Task id: 6132 -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV 272/272 272/272 ILK -1 302/302 301/302 SNB 303/303 303/303 IVB -1 338/338 337/338 BYT -1 287/287 286/287 HSW 361/361 361/361 BDW 308/308 308/308 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied *ILK igt@kms_pipe_crc_basic@bad-nb-words-3 PASS(2) DMESG_WARN(1)PASS(1) (dmesg patch applied)drm:drm_edid_block_valid[drm]]*ERROR*EDID_checksum_is_invalid,remainder_is@EDID checksum is .* remainder is *IVB igt@gem_pwrite_pread@snooped-pwrite-blt-cpu_mmap-performance PASS(2) DMESG_WARN(1)PASS(1) (dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle *BYT igt@gem_exec_bad_domains@conflicting-write-domain PASS(19) FAIL(1)PASS(1) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: reset drm state backpointer in crtc_state 2015-04-06 18:30 [PATCH] drm/i915: reset drm state backpointer in crtc_state Chandra Konduru 2015-04-06 20:52 ` shuang.he @ 2015-04-07 9:02 ` Jani Nikula 2015-04-07 18:48 ` Konduru, Chandra 2015-04-08 8:22 ` Daniel Vetter 2 siblings, 1 reply; 7+ messages in thread From: Jani Nikula @ 2015-04-07 9:02 UTC (permalink / raw) To: Chandra Konduru, intel-gfx On Mon, 06 Apr 2015, Chandra Konduru <chandra.konduru@intel.com> wrote: > At end of intel_crtc_set_config, reset crtc_state's > drm_state back pointer to null. This does not tell me anything that reading the patch already didn't. Please explain *why* this is needed in the commit message. What breaks without it? If this fixes a regression, please indicate which commit regressed. BR, Jani. > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index c84926b..f9c2e4d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12451,8 +12451,10 @@ fail: > } > > out_config: > - if (state) > + if (state) { > drm_atomic_state_free(state); > + to_intel_crtc(set->crtc)->config->base.state = NULL; > + } > > intel_set_config_free(config); > return ret; > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: reset drm state backpointer in crtc_state 2015-04-07 9:02 ` Jani Nikula @ 2015-04-07 18:48 ` Konduru, Chandra 2015-04-08 8:19 ` Daniel Vetter 0 siblings, 1 reply; 7+ messages in thread From: Konduru, Chandra @ 2015-04-07 18:48 UTC (permalink / raw) To: Jani Nikula, intel-gfx > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > Sent: Tuesday, April 07, 2015 2:02 AM > To: Konduru, Chandra; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: reset drm state backpointer in > crtc_state > > On Mon, 06 Apr 2015, Chandra Konduru <chandra.konduru@intel.com> wrote: > > At end of intel_crtc_set_config, reset crtc_state's drm_state back > > pointer to null. > > This does not tell me anything that reading the patch already didn't. Please > explain *why* this is needed in the commit message. What breaks without it? If > this fixes a regression, please indicate which commit regressed. Once atomic transaction is done, live crtc_state (i.e., intel_crtc->config) is carrying back pointer to drm_atomic_state which is freed. When a new non-atomic transaction is made (crtc_disable triggered off from set_mode), this stale pointer is carried into that transaction. Depending on timing, this causes issue to scaler feature that I am working if panel fit to be disabled during crtc_disable. It has potential to cause issues to wm work that Matt is doing, but not sure. > > BR, > Jani. > > > > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index c84926b..f9c2e4d 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -12451,8 +12451,10 @@ fail: > > } > > > > out_config: > > - if (state) > > + if (state) { > > drm_atomic_state_free(state); > > + to_intel_crtc(set->crtc)->config->base.state = NULL; > > + } > > > > intel_set_config_free(config); > > return ret; > > -- > > 1.7.9.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: reset drm state backpointer in crtc_state 2015-04-07 18:48 ` Konduru, Chandra @ 2015-04-08 8:19 ` Daniel Vetter 2015-04-08 16:26 ` Konduru, Chandra 0 siblings, 1 reply; 7+ messages in thread From: Daniel Vetter @ 2015-04-08 8:19 UTC (permalink / raw) To: Konduru, Chandra; +Cc: intel-gfx On Tue, Apr 07, 2015 at 06:48:15PM +0000, Konduru, Chandra wrote: > > > -----Original Message----- > > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > > Sent: Tuesday, April 07, 2015 2:02 AM > > To: Konduru, Chandra; intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: reset drm state backpointer in > > crtc_state > > > > On Mon, 06 Apr 2015, Chandra Konduru <chandra.konduru@intel.com> wrote: > > > At end of intel_crtc_set_config, reset crtc_state's drm_state back > > > pointer to null. > > > > This does not tell me anything that reading the patch already didn't. Please > > explain *why* this is needed in the commit message. What breaks without it? If > > this fixes a regression, please indicate which commit regressed. > > Once atomic transaction is done, live crtc_state (i.e., intel_crtc->config) is > carrying back pointer to drm_atomic_state which is freed. When a new > non-atomic transaction is made (crtc_disable triggered off from set_mode), > this stale pointer is carried into that transaction. > Depending on timing, this causes issue to scaler feature that I am working > if panel fit to be disabled during crtc_disable. You shouldn't ever be chasing the state backpointer of a committed state. It's ok to clean it up, but chasing that pointer itself is a bug. Why does the scaler code look at that pointer? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: reset drm state backpointer in crtc_state 2015-04-08 8:19 ` Daniel Vetter @ 2015-04-08 16:26 ` Konduru, Chandra 0 siblings, 0 replies; 7+ messages in thread From: Konduru, Chandra @ 2015-04-08 16:26 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Wednesday, April 08, 2015 1:20 AM > To: Konduru, Chandra > Cc: Jani Nikula; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: reset drm state backpointer in > crtc_state > > On Tue, Apr 07, 2015 at 06:48:15PM +0000, Konduru, Chandra wrote: > > > > > -----Original Message----- > > > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > > > Sent: Tuesday, April 07, 2015 2:02 AM > > > To: Konduru, Chandra; intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: reset drm state > > > backpointer in crtc_state > > > > > > On Mon, 06 Apr 2015, Chandra Konduru <chandra.konduru@intel.com> > wrote: > > > > At end of intel_crtc_set_config, reset crtc_state's drm_state back > > > > pointer to null. > > > > > > This does not tell me anything that reading the patch already > > > didn't. Please explain *why* this is needed in the commit message. > > > What breaks without it? If this fixes a regression, please indicate which > commit regressed. > > > > Once atomic transaction is done, live crtc_state (i.e., > > intel_crtc->config) is carrying back pointer to drm_atomic_state which > > is freed. When a new non-atomic transaction is made (crtc_disable > > triggered off from set_mode), this stale pointer is carried into that transaction. > > Depending on timing, this causes issue to scaler feature that I am > > working if panel fit to be disabled during crtc_disable. > > You shouldn't ever be chasing the state backpointer of a committed state. > It's ok to clean it up, but chasing that pointer itself is a bug. Why does the scaler > code look at that pointer? While setting up scalers, it looks at that pointer to know other scaler users. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: reset drm state backpointer in crtc_state 2015-04-06 18:30 [PATCH] drm/i915: reset drm state backpointer in crtc_state Chandra Konduru 2015-04-06 20:52 ` shuang.he 2015-04-07 9:02 ` Jani Nikula @ 2015-04-08 8:22 ` Daniel Vetter 2 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2015-04-08 8:22 UTC (permalink / raw) To: Chandra Konduru; +Cc: intel-gfx On Mon, Apr 06, 2015 at 11:30:29AM -0700, Chandra Konduru wrote: > At end of intel_crtc_set_config, reset crtc_state's > drm_state back pointer to null. > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index c84926b..f9c2e4d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12451,8 +12451,10 @@ fail: > } > > out_config: > - if (state) > + if (state) { > drm_atomic_state_free(state); > + to_intel_crtc(set->crtc)->config->base.state = NULL; This should be done in intel_crtc_set_state instead of just in this case here. -Daniel > + } > > intel_set_config_free(config); > return ret; > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-04-08 16:27 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-06 18:30 [PATCH] drm/i915: reset drm state backpointer in crtc_state Chandra Konduru 2015-04-06 20:52 ` shuang.he 2015-04-07 9:02 ` Jani Nikula 2015-04-07 18:48 ` Konduru, Chandra 2015-04-08 8:19 ` Daniel Vetter 2015-04-08 16:26 ` Konduru, Chandra 2015-04-08 8:22 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.