On Thu, 2016-01-21 at 13:52 +0200, Joonas Lahtinen wrote:
> On to, 2016-01-14 at 18:50 +0200, Imre Deak wrote:
> [...]
> Below patch looks fine. Just that I'd use s/if_enabled/if_in_use/ to
> match the PM API better. We're already doing too much of a good job
> to
> having many names for same thing.
We do have two separate states that we handle separately in both
frameworks:
- enabled/active: the HW is powered-on at the moment
- in_use: the HW is powered-on _and_ we hold a reference
So imo it makes sense to make this distinction in the function names
too.
--Imre
> Regards, Joonas
>
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 1f9a368..907377dc 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1910,13 +1910,16 @@ bool
> > intel_ddi_connector_get_hw_state(struct
> > intel_connector *intel_connector)
> > enum transcoder cpu_transcoder;
> > enum intel_display_power_domain power_domain;
> > uint32_t tmp;
> > + bool ret;
> >
> > power_domain =
> > intel_display_port_power_domain(intel_encoder);
> > - if (!intel_display_power_is_enabled(dev_priv,
> > power_domain))
> > + if (!intel_display_power_get_if_enabled(dev_priv,
> > power_domain))
> > return false;
> >
> > - if (!intel_encoder->get_hw_state(intel_encoder, &pipe))
> > - return false;
> > + if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> > + ret = false;
> > + goto out;
> > + }
> >
> > if (port == PORT_A)
> > cpu_transcoder = TRANSCODER_EDP;
> > @@ -1928,23 +1931,30 @@ bool
> > intel_ddi_connector_get_hw_state(struct
> > intel_connector *intel_connector)
> > switch (tmp & TRANS_DDI_MODE_SELECT_MASK) {
> > case TRANS_DDI_MODE_SELECT_HDMI:
> > case TRANS_DDI_MODE_SELECT_DVI:
> > - return (type == DRM_MODE_CONNECTOR_HDMIA);
> > + ret = type == DRM_MODE_CONNECTOR_HDMIA;
> > + goto out;
> >
> > case TRANS_DDI_MODE_SELECT_DP_SST:
> > - if (type == DRM_MODE_CONNECTOR_eDP)
> > - return true;
> > - return (type == DRM_MODE_CONNECTOR_DisplayPort);
> > + ret = type == DRM_MODE_CONNECTOR_eDP ||
> > + type == DRM_MODE_CONNECTOR_DisplayPort;
> > + goto out;
> > case TRANS_DDI_MODE_SELECT_DP_MST:
> > /* if the transcoder is in MST state then
> > * connector isn't connected */
> > - return false;
> > + ret = false;
> > + goto out;
> >
> > case TRANS_DDI_MODE_SELECT_FDI:
> > - return (type == DRM_MODE_CONNECTOR_VGA);
> > + ret = type == DRM_MODE_CONNECTOR_VGA;
> > + goto out;
> >
> > default:
> > - return false;
> > + ret = false;
> > }
> > +out:
> > + intel_display_power_put(dev_priv, power_domain);
> > +
> > + return ret;
> > }
> >
> > bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 059b46e..3c84159 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1456,6 +1456,8 @@ bool __intel_display_power_is_enabled(struct
> > drm_i915_private *dev_priv,
> > enum
> > intel_display_power_domain domain);
> > void intel_display_power_get(struct drm_i915_private *dev_priv,
> > enum intel_display_power_domain
> > domain);
> > +bool intel_display_power_get_if_enabled(struct drm_i915_private
> > *dev_priv,
> > + enum
> > intel_display_power_domain domain);
> > void intel_display_power_put(struct drm_i915_private *dev_priv,
> > enum intel_display_power_domain
> > domain);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index bbca527..6c4f170 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -1470,6 +1470,17 @@ void intel_display_power_get(struct
> > drm_i915_private *dev_priv,
> > mutex_unlock(&power_domains->lock);
> > }
> >
> > +bool intel_display_power_get_if_enabled(struct drm_i915_private
> > *dev_priv,
> > + enum
> > intel_display_power_domain domain)
> > +{
> > + if (!intel_display_power_is_enabled(dev_priv, domain))
> > + return false;
> > +
> > + intel_display_power_get(dev_priv, domain);
> > +
> > + return true;
> > +}
> > +
> > /**
> > * intel_display_power_put - release a power domain reference
> > * @dev_priv: i915 device instance
> >
> > --Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
