| Message ID | 1373579105-1732-8-git-send-email-rodrigo.vivi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 11, 2013 at 06:45:01PM -0300, Rodrigo Vivi wrote: > @@ -1602,6 +1611,26 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) > DRM_ERROR("Timed out waiting for PSR Idle State\n"); > } > > +void intel_edp_psr_update(struct drm_device *dev) > +{ > + struct intel_encoder *encoder; > + struct intel_dp *intel_dp = NULL; > + > + list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) > + if (encoder->type == INTEL_OUTPUT_EDP) { How many eDP are you planning to allow on the system? We already have precedence for the presumption of a single (integrated) panel on a device, maybe we can add the logic there (i.e. stash a back pointer in this case)? We could then also do a debugfs/i915_panel_info where we can inspect various details normally associated with the panels (PSR status, backlight interface, eDP vs LVDS, etc). -Chris
On Mon, Jul 15, 2013 at 11:00 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Thu, Jul 11, 2013 at 06:45:01PM -0300, Rodrigo Vivi wrote: >> @@ -1602,6 +1611,26 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) >> DRM_ERROR("Timed out waiting for PSR Idle State\n"); >> } >> >> +void intel_edp_psr_update(struct drm_device *dev) >> +{ >> + struct intel_encoder *encoder; >> + struct intel_dp *intel_dp = NULL; >> + >> + list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) >> + if (encoder->type == INTEL_OUTPUT_EDP) { > > How many eDP are you planning to allow on the system? We already have > precedence for the presumption of a single (integrated) panel on a device, > maybe we can add the logic there (i.e. stash a back pointer in this case)? That is a good question... I asked it myself many times when I was trying to get intel_dp with edp from dev... For the first version I just run the loop getting any intel_dp with edp since we have this assumption of only one edp, but then I thought about that convertibles with 2 panels and since in hsw we can have edp on port D I decided to let the implementation more generic as possible although I know we won't have this case... at least not any time soon. > > We could then also do a debugfs/i915_panel_info where we can inspect > various details normally associated with the panels (PSR status, backlight > interface, eDP vs LVDS, etc). I liked this idea... will have in mind for later > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre Thanks -- Rodrigo Vivi Blog: http://blog.vivi.eng.br
On Mon, Jul 15, 2013 at 05:21:12PM -0300, Rodrigo Vivi wrote: > On Mon, Jul 15, 2013 at 11:00 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Thu, Jul 11, 2013 at 06:45:01PM -0300, Rodrigo Vivi wrote: > >> @@ -1602,6 +1611,26 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) > >> DRM_ERROR("Timed out waiting for PSR Idle State\n"); > >> } > >> > >> +void intel_edp_psr_update(struct drm_device *dev) > >> +{ > >> + struct intel_encoder *encoder; > >> + struct intel_dp *intel_dp = NULL; > >> + > >> + list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) > >> + if (encoder->type == INTEL_OUTPUT_EDP) { > > > > How many eDP are you planning to allow on the system? We already have > > precedence for the presumption of a single (integrated) panel on a device, > > maybe we can add the logic there (i.e. stash a back pointer in this case)? > > That is a good question... I asked it myself many times when I was > trying to get intel_dp with edp from dev... > For the first version I just run the loop getting any intel_dp with > edp since we have this assumption of only one edp, > but then I thought about that convertibles with 2 panels and since in > hsw we can have edp on port D I decided to let the implementation > more generic as possible although I know we won't have this case... at > least not any time soon. The way I nowadays solve such a conundrum is to shovel a bit of metadata (like psr_capable_sink) into pipe_config and let encoders fill it out appropriately in their ->compute_config functions. Most of the "walk over all encoders and noodle int their innards" have disappeared through that. -Daniel
2013/7/11 Rodrigo Vivi <rodrigo.vivi@gmail.com>: > Required function to disable PSR when going to console mode. > But also can be used whenever PSR mode entry conditions changed. > > v2: Add it before PSR Hook. Update function not really been called yet. > v3: Fix coding style detected by checkpatch by Paulo Zanoni. > v4: do_enable must be static as Paulo noticed. > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> Even though we can improve the loops and other things with pipe_config, this patch seems to work, so: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> We can still do the reworks later. > --- > drivers/gpu/drm/i915/intel_dp.c | 31 ++++++++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 2 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index c0bd887..c0870a69 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1568,7 +1568,7 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) > return true; > } > > -void intel_edp_psr_enable(struct intel_dp *intel_dp) > +static void intel_edp_psr_do_enable(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > > @@ -1586,6 +1586,15 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp) > intel_edp_psr_enable_source(intel_dp); > } > > +void intel_edp_psr_enable(struct intel_dp *intel_dp) > +{ > + struct drm_device *dev = intel_dp_to_dev(intel_dp); > + > + if (intel_edp_psr_match_conditions(intel_dp) && > + !intel_edp_is_psr_enabled(dev)) > + intel_edp_psr_do_enable(intel_dp); > +} > + > void intel_edp_psr_disable(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > @@ -1602,6 +1611,26 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) > DRM_ERROR("Timed out waiting for PSR Idle State\n"); > } > > +void intel_edp_psr_update(struct drm_device *dev) > +{ > + struct intel_encoder *encoder; > + struct intel_dp *intel_dp = NULL; > + > + list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) > + if (encoder->type == INTEL_OUTPUT_EDP) { > + intel_dp = enc_to_intel_dp(&encoder->base); > + > + if (!is_edp_psr(intel_dp)) > + return; > + > + if (!intel_edp_psr_match_conditions(intel_dp)) > + intel_edp_psr_disable(intel_dp); > + else > + if (!intel_edp_is_psr_enabled(dev)) > + intel_edp_psr_do_enable(intel_dp); > + } > +} > + > static void intel_disable_dp(struct intel_encoder *encoder) > { > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index ff36a40..40e955d 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -837,5 +837,6 @@ extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev, > > extern void intel_edp_psr_enable(struct intel_dp *intel_dp); > extern void intel_edp_psr_disable(struct intel_dp *intel_dp); > +extern void intel_edp_psr_update(struct drm_device *dev); > > #endif /* __INTEL_DRV_H__ */ > -- > 1.7.11.7 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c0bd887..c0870a69 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1568,7 +1568,7 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) return true; } -void intel_edp_psr_enable(struct intel_dp *intel_dp) +static void intel_edp_psr_do_enable(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); @@ -1586,6 +1586,15 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp) intel_edp_psr_enable_source(intel_dp); } +void intel_edp_psr_enable(struct intel_dp *intel_dp) +{ + struct drm_device *dev = intel_dp_to_dev(intel_dp); + + if (intel_edp_psr_match_conditions(intel_dp) && + !intel_edp_is_psr_enabled(dev)) + intel_edp_psr_do_enable(intel_dp); +} + void intel_edp_psr_disable(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); @@ -1602,6 +1611,26 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) DRM_ERROR("Timed out waiting for PSR Idle State\n"); } +void intel_edp_psr_update(struct drm_device *dev) +{ + struct intel_encoder *encoder; + struct intel_dp *intel_dp = NULL; + + list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) + if (encoder->type == INTEL_OUTPUT_EDP) { + intel_dp = enc_to_intel_dp(&encoder->base); + + if (!is_edp_psr(intel_dp)) + return; + + if (!intel_edp_psr_match_conditions(intel_dp)) + intel_edp_psr_disable(intel_dp); + else + if (!intel_edp_is_psr_enabled(dev)) + intel_edp_psr_do_enable(intel_dp); + } +} + static void intel_disable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ff36a40..40e955d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -837,5 +837,6 @@ extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev, extern void intel_edp_psr_enable(struct intel_dp *intel_dp); extern void intel_edp_psr_disable(struct intel_dp *intel_dp); +extern void intel_edp_psr_update(struct drm_device *dev); #endif /* __INTEL_DRV_H__ */
Required function to disable PSR when going to console mode. But also can be used whenever PSR mode entry conditions changed. v2: Add it before PSR Hook. Update function not really been called yet. v3: Fix coding style detected by checkpatch by Paulo Zanoni. v4: do_enable must be static as Paulo noticed. Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> --- drivers/gpu/drm/i915/intel_dp.c | 31 ++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 31 insertions(+), 1 deletion(-)