| Message ID | 1373579105-1732-10-git-send-email-rodrigo.vivi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Chris, could you please review this specific one or give you ack here? Thanks On Thu, Jul 11, 2013 at 6:45 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > This global value allows userspace know when PSR is enabled. > > This will allow userspace emit more busy_ioctl when doing directly copy_area > operations through scanout allowing forced psr exit. > > v2: Check for PSR enabled instead of active. (by Chris Wilson) > v3: Use existing intel_edp_is_psr_enabled function. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> > --- > drivers/gpu/drm/i915/i915_dma.c | 3 +++ > drivers/gpu/drm/i915/intel_dp.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h | 1 + > include/uapi/drm/i915_drm.h | 1 + > 4 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 6ce9033..1e5dd1c 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1000,6 +1000,9 @@ static int i915_getparam(struct drm_device *dev, void *data, > case I915_PARAM_HAS_EXEC_HANDLE_LUT: > value = 1; > break; > + case I915_PARAM_PSR_ENABLED: > + value = intel_edp_is_psr_enabled(dev); > + break; > default: > DRM_DEBUG("Unknown parameter %d\n", param->param); > return -EINVAL; > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index c0defaf..3c9473c 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1383,7 +1383,7 @@ static bool is_edp_psr(struct intel_dp *intel_dp) > intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED; > } > > -static bool intel_edp_is_psr_enabled(struct drm_device *dev) > +bool intel_edp_is_psr_enabled(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 40e955d..0f52362 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -838,5 +838,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); > +extern bool intel_edp_is_psr_enabled(struct drm_device *dev); > > #endif /* __INTEL_DRV_H__ */ > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 923ed7f..a5db73b 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -310,6 +310,7 @@ typedef struct drm_i915_irq_wait { > #define I915_PARAM_HAS_PINNED_BATCHES 24 > #define I915_PARAM_HAS_EXEC_NO_RELOC 25 > #define I915_PARAM_HAS_EXEC_HANDLE_LUT 26 > +#define I915_PARAM_PSR_ENABLED 27 > > typedef struct drm_i915_getparam { > int param; > -- > 1.7.11.7 >
On Wed, Jul 17, 2013 at 02:46:52PM -0300, Rodrigo Vivi wrote: > Hi Chris, > > could you please review this specific one or give you ack here? Didn't see anything wrong with it. The only caveat I have is that the GETPARAM must be accurate immediately following a setcrtc. If you can guarrantee that is true, you can have my Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> and if Daniel delays, ask him to reserve the PARAM number. -Chris
On Wed, Jul 17, 2013 at 5:18 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Wed, Jul 17, 2013 at 02:46:52PM -0300, Rodrigo Vivi wrote: >> Hi Chris, >> >> could you please review this specific one or give you ack here? > > Didn't see anything wrong with it. The only caveat I have is that the > GETPARAM must be accurate immediately following a setcrtc. To be truly honest I have no idea, mainly when we alternate with fbcon updating psr state at set_base. Could you please also review subsequent patches in this series... 10 and 11. I think 11 answer this question... Another alternative would be using i915_enable_psr + i915_powersave check instead of reading the register for current enabled status. > If you can > guarrantee that is true, you can have my > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > and if Daniel delays, ask him to reserve the PARAM number. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Rodrigo Vivi Blog: http://blog.vivi.eng.br
On Wed, Jul 17, 2013 at 06:01:03PM -0300, Rodrigo Vivi wrote: > On Wed, Jul 17, 2013 at 5:18 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Wed, Jul 17, 2013 at 02:46:52PM -0300, Rodrigo Vivi wrote: > >> Hi Chris, > >> > >> could you please review this specific one or give you ack here? > > > > Didn't see anything wrong with it. The only caveat I have is that the > > GETPARAM must be accurate immediately following a setcrtc. > > To be truly honest I have no idea, mainly when we alternate with fbcon > updating psr state at set_base. > Could you please also review subsequent patches in this series... 10 and 11. > I think 11 answer this question... If it is not clear by this point, and the changelog doesn't make it clear, then something is missing from this patch. Hint ;-) -Chris
On Wed, Jul 17, 2013 at 10:08:34PM +0100, Chris Wilson wrote: > On Wed, Jul 17, 2013 at 06:01:03PM -0300, Rodrigo Vivi wrote: > > On Wed, Jul 17, 2013 at 5:18 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > On Wed, Jul 17, 2013 at 02:46:52PM -0300, Rodrigo Vivi wrote: > > >> Hi Chris, > > >> > > >> could you please review this specific one or give you ack here? > > > > > > Didn't see anything wrong with it. The only caveat I have is that the > > > GETPARAM must be accurate immediately following a setcrtc. > > > > To be truly honest I have no idea, mainly when we alternate with fbcon > > updating psr state at set_base. > > Could you please also review subsequent patches in this series... 10 and 11. > > I think 11 answer this question... > > If it is not clear by this point, and the changelog doesn't make it > clear, then something is missing from this patch. Hint ;-) I'll punt on userspace interface changes, at least until we've figured out a clear picture how to do this. -Daniel
On Thu, Jul 18, 2013 at 5:24 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Jul 17, 2013 at 10:08:34PM +0100, Chris Wilson wrote: >> On Wed, Jul 17, 2013 at 06:01:03PM -0300, Rodrigo Vivi wrote: >> > On Wed, Jul 17, 2013 at 5:18 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: >> > > On Wed, Jul 17, 2013 at 02:46:52PM -0300, Rodrigo Vivi wrote: >> > >> Hi Chris, >> > >> >> > >> could you please review this specific one or give you ack here? >> > > >> > > Didn't see anything wrong with it. The only caveat I have is that the >> > > GETPARAM must be accurate immediately following a setcrtc. >> > >> > To be truly honest I have no idea, mainly when we alternate with fbcon >> > updating psr state at set_base. >> > Could you please also review subsequent patches in this series... 10 and 11. >> > I think 11 answer this question... >> >> If it is not clear by this point, and the changelog doesn't make it >> clear, then something is missing from this patch. Hint ;-) > > I'll punt on userspace interface changes, at least until we've figured out > a clear picture how to do this. agreed > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Rodrigo Vivi Blog: http://blog.vivi.eng.br
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 6ce9033..1e5dd1c 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1000,6 +1000,9 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_HAS_EXEC_HANDLE_LUT: value = 1; break; + case I915_PARAM_PSR_ENABLED: + value = intel_edp_is_psr_enabled(dev); + break; default: DRM_DEBUG("Unknown parameter %d\n", param->param); return -EINVAL; diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c0defaf..3c9473c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1383,7 +1383,7 @@ static bool is_edp_psr(struct intel_dp *intel_dp) intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED; } -static bool intel_edp_is_psr_enabled(struct drm_device *dev) +bool intel_edp_is_psr_enabled(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 40e955d..0f52362 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -838,5 +838,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); +extern bool intel_edp_is_psr_enabled(struct drm_device *dev); #endif /* __INTEL_DRV_H__ */ diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 923ed7f..a5db73b 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -310,6 +310,7 @@ typedef struct drm_i915_irq_wait { #define I915_PARAM_HAS_PINNED_BATCHES 24 #define I915_PARAM_HAS_EXEC_NO_RELOC 25 #define I915_PARAM_HAS_EXEC_HANDLE_LUT 26 +#define I915_PARAM_PSR_ENABLED 27 typedef struct drm_i915_getparam { int param;
This global value allows userspace know when PSR is enabled. This will allow userspace emit more busy_ioctl when doing directly copy_area operations through scanout allowing forced psr exit. v2: Check for PSR enabled instead of active. (by Chris Wilson) v3: Use existing intel_edp_is_psr_enabled function. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> --- drivers/gpu/drm/i915/i915_dma.c | 3 +++ drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 1 + include/uapi/drm/i915_drm.h | 1 + 4 files changed, 6 insertions(+), 1 deletion(-)