| Message ID | 20180314223617.20122-2-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 14, 2018 at 03:36:13PM -0700, José Roberto de Souza wrote: > Move to only one place the sink requirements that the actual driver > needs to enable PSR2. > > Also intel_psr2_config_valid() is called every time the crtc config > is computed, wasting some time every time it was checking for > Y coordinate requirement. > > This allow us to nuke y_cord_support and some of VSC setup code that > was handling a scenario that would never happen(PSR2 without Y > coordinate). > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 - > drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++-------------------- > 2 files changed, 28 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8a584273f897..d4bc8d18f56c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -603,7 +603,6 @@ struct i915_psr { > unsigned busy_frontbuffer_bits; > bool psr2_support; > bool link_standby; > - bool y_cord_support; > bool colorimetry_support; > bool alpm; > bool has_hw_tracking; > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index 9811f5f0bc75..62d97d5576d1 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp *intel_dp) > intel_display_power_put(dev_priv, psr_aux_domain(intel_dp)); > } > > -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp) > +static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp) > { > uint8_t psr_caps = 0; > > @@ -137,22 +137,38 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) > > if (INTEL_GEN(dev_priv) >= 9 && > (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) { > - uint8_t frame_sync_cap; > + uint8_t frame_sync_cap, y_coord_req; > > dev_priv->psr.sink_support = true; > + > + /* PSR2 needs frame sync to do selective updates */ > if (drm_dp_dpcd_readb(&intel_dp->aux, > DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > &frame_sync_cap) != 1) > frame_sync_cap = 0; > frame_sync_cap = (frame_sync_cap & DP_AUX_FRAME_SYNC_CAP); > - /* PSR2 needs frame sync as well */ > - dev_priv->psr.psr2_support = frame_sync_cap; > - DRM_DEBUG_KMS("PSR2 %s on sink", > - dev_priv->psr.psr2_support ? "supported" : "not supported"); > + > + /* > + * FIXME: Enable PSR2 only for y-coordinate PSR2 panels > + * After GTC implementation, remove this restriction. I believe we should remove the FIXME and just comment out that non y-coordinate would need GTC that we currently don't implement. later if we change our minds and decided to add that support we revisit here but without a FIXME poluting code for something we don't plan to support for now. > + * > + * All panels that supports PSR version 3 that is PSRv3? is there really something formal about that? > + * PSR2 + Y-coordinate support can handle Y coordinates in VSC > + * but we are only sure that it is going to be used when > + * required by the panel. This way panel is capable to do > + * selective update even without a valid aux frame sync. > + */ > + y_coord_req = intel_dp_get_y_coord_required(intel_dp); > + > + dev_priv->psr.psr2_support = frame_sync_cap && y_coord_req; > + if (dev_priv->psr.psr2_support) > + DRM_DEBUG_KMS("PSR2 supported on sink\n"); > + else > + DRM_DEBUG_KMS("PSR2 not supported on sink" > + "(frame sync: %d Y-coord required: %d)\n", > + frame_sync_cap, y_coord_req); > > if (dev_priv->psr.psr2_support) { > - dev_priv->psr.y_cord_support = > - intel_dp_get_y_cord_status(intel_dp); > dev_priv->psr.colorimetry_support = > intel_dp_get_colorimetry_status(intel_dp); > dev_priv->psr.alpm = > @@ -198,16 +214,12 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp, > memset(&psr_vsc, 0, sizeof(psr_vsc)); > psr_vsc.sdp_header.HB0 = 0; > psr_vsc.sdp_header.HB1 = 0x7; > - if (dev_priv->psr.colorimetry_support && > - dev_priv->psr.y_cord_support) { > + if (dev_priv->psr.colorimetry_support) { > psr_vsc.sdp_header.HB2 = 0x5; > psr_vsc.sdp_header.HB3 = 0x13; > - } else if (dev_priv->psr.y_cord_support) { > + } else { > psr_vsc.sdp_header.HB2 = 0x4; > psr_vsc.sdp_header.HB3 = 0xe; > - } else { > - psr_vsc.sdp_header.HB2 = 0x3; > - psr_vsc.sdp_header.HB3 = 0xc; > } > } else { > /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ > @@ -478,15 +490,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, > return false; > } > > - /* > - * FIXME:enable psr2 only for y-cordinate psr2 panels > - * After gtc implementation , remove this restriction. > - */ > - if (!dev_priv->psr.y_cord_support) { > - DRM_DEBUG_KMS("PSR2 not enabled, panel does not support Y coordinate\n"); > - return false; > - } > - > return true; > } > > @@ -586,14 +589,12 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, > struct drm_device *dev = dig_port->base.base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > - u32 chicken; > > psr_aux_io_power_get(intel_dp); > > if (dev_priv->psr.psr2_support) { > - chicken = PSR2_VSC_ENABLE_PROG_HEADER; > - if (dev_priv->psr.y_cord_support) > - chicken |= PSR2_ADD_VERTICAL_LINE_COUNT; > + u32 chicken = PSR2_VSC_ENABLE_PROG_HEADER > + | PSR2_ADD_VERTICAL_LINE_COUNT; > I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken); > > I915_WRITE(EDP_PSR_DEBUG, > -- > 2.16.2 >
On Thu, 2018-03-15 at 17:35 -0700, Rodrigo Vivi wrote: > On Wed, Mar 14, 2018 at 03:36:13PM -0700, José Roberto de Souza > wrote: > > Move to only one place the sink requirements that the actual driver > > needs to enable PSR2. > > > > Also intel_psr2_config_valid() is called every time the crtc config > > is computed, wasting some time every time it was checking for > > Y coordinate requirement. > > > > This allow us to nuke y_cord_support and some of VSC setup code > > that > > was handling a scenario that would never happen(PSR2 without Y > > coordinate). > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 1 - > > drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++------ > > -------------- > > 2 files changed, 28 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 8a584273f897..d4bc8d18f56c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -603,7 +603,6 @@ struct i915_psr { > > unsigned busy_frontbuffer_bits; > > bool psr2_support; > > bool link_standby; > > - bool y_cord_support; > > bool colorimetry_support; > > bool alpm; > > bool has_hw_tracking; > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index 9811f5f0bc75..62d97d5576d1 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp > > *intel_dp) > > intel_display_power_put(dev_priv, > > psr_aux_domain(intel_dp)); > > } > > > > -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp) > > +static bool intel_dp_get_y_coord_required(struct intel_dp > > *intel_dp) > > { > > uint8_t psr_caps = 0; > > > > @@ -137,22 +137,38 @@ void intel_psr_init_dpcd(struct intel_dp > > *intel_dp) > > > > if (INTEL_GEN(dev_priv) >= 9 && > > (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) { > > - uint8_t frame_sync_cap; > > + uint8_t frame_sync_cap, y_coord_req; > > > > dev_priv->psr.sink_support = true; > > + > > + /* PSR2 needs frame sync to do selective updates > > */ > > if (drm_dp_dpcd_readb(&intel_dp->aux, > > DP_SINK_DEVICE_AUX_FRAME_SYN > > C_CAP, > > &frame_sync_cap) != 1) > > frame_sync_cap = 0; > > frame_sync_cap = (frame_sync_cap & > > DP_AUX_FRAME_SYNC_CAP); > > - /* PSR2 needs frame sync as well */ > > - dev_priv->psr.psr2_support = frame_sync_cap; > > - DRM_DEBUG_KMS("PSR2 %s on sink", > > - dev_priv->psr.psr2_support ? > > "supported" : "not supported"); > > + > > + /* > > + * FIXME: Enable PSR2 only for y-coordinate PSR2 > > panels > > + * After GTC implementation, remove this > > restriction. > > I believe we should remove the FIXME and just comment out that non y- > coordinate > would need GTC that we currently don't implement. > > later if we change our minds and decided to add that support we > revisit here > but without a FIXME poluting code for something we don't plan to > support for now. > > > + * > > + * All panels that supports PSR version 3 that is > > PSRv3? is there really something formal about that? Yes, from eDP spec: PSR Support and Version 00h = Panel Self Refresh Capability is not supported (default). If PSR/PSR2 is supported, the SET_POWER_CAPABLE bit in the EDP_GENERAL_CAPABILITY_1 register (DPCD Address 00701h, bit 7) must be set to 1. 01h = Panel Self Refresh capability is supported, PSR version is 01h (PSR). 02h = Panel Self Refresh with Selective Update (SU) capability is supported, PSR version is 02h (PSR2). Y-coordinates for SU are not supported. Supported by eDP v1.4 (and higher). 03h = Panel Self Refresh with SU capability and Y-coordinate supported, PSR version is 03h (PSR2 + Y-coordinate). Supported by eDP v1.4a (and higher). > > > + * PSR2 + Y-coordinate support can handle Y > > coordinates in VSC > > + * but we are only sure that it is going to be > > used when > > + * required by the panel. This way panel is > > capable to do > > + * selective update even without a valid aux frame > > sync. > > + */ > > + y_coord_req = > > intel_dp_get_y_coord_required(intel_dp); > > + > > + dev_priv->psr.psr2_support = frame_sync_cap && > > y_coord_req; > > + if (dev_priv->psr.psr2_support) > > + DRM_DEBUG_KMS("PSR2 supported on sink\n"); > > + else > > + DRM_DEBUG_KMS("PSR2 not supported on sink" > > + "(frame sync: %d Y-coord > > required: %d)\n", > > + frame_sync_cap, > > y_coord_req); > > > > if (dev_priv->psr.psr2_support) { > > - dev_priv->psr.y_cord_support = > > - intel_dp_get_y_cord_status(intel_d > > p); > > dev_priv->psr.colorimetry_support = > > intel_dp_get_colorimetry_status(in > > tel_dp); > > dev_priv->psr.alpm = > > @@ -198,16 +214,12 @@ static void hsw_psr_setup_vsc(struct intel_dp > > *intel_dp, > > memset(&psr_vsc, 0, sizeof(psr_vsc)); > > psr_vsc.sdp_header.HB0 = 0; > > psr_vsc.sdp_header.HB1 = 0x7; > > - if (dev_priv->psr.colorimetry_support && > > - dev_priv->psr.y_cord_support) { > > + if (dev_priv->psr.colorimetry_support) { > > psr_vsc.sdp_header.HB2 = 0x5; > > psr_vsc.sdp_header.HB3 = 0x13; > > - } else if (dev_priv->psr.y_cord_support) { > > + } else { > > psr_vsc.sdp_header.HB2 = 0x4; > > psr_vsc.sdp_header.HB3 = 0xe; > > - } else { > > - psr_vsc.sdp_header.HB2 = 0x3; > > - psr_vsc.sdp_header.HB3 = 0xc; > > } > > } else { > > /* Prepare VSC packet as per EDP 1.3 spec, Table > > 3.10 */ > > @@ -478,15 +490,6 @@ static bool intel_psr2_config_valid(struct > > intel_dp *intel_dp, > > return false; > > } > > > > - /* > > - * FIXME:enable psr2 only for y-cordinate psr2 panels > > - * After gtc implementation , remove this restriction. > > - */ > > - if (!dev_priv->psr.y_cord_support) { > > - DRM_DEBUG_KMS("PSR2 not enabled, panel does not > > support Y coordinate\n"); > > - return false; > > - } > > - > > return true; > > } > > > > @@ -586,14 +589,12 @@ static void hsw_psr_enable_source(struct > > intel_dp *intel_dp, > > struct drm_device *dev = dig_port->base.base.dev; > > struct drm_i915_private *dev_priv = to_i915(dev); > > enum transcoder cpu_transcoder = crtc_state- > > >cpu_transcoder; > > - u32 chicken; > > > > psr_aux_io_power_get(intel_dp); > > > > if (dev_priv->psr.psr2_support) { > > - chicken = PSR2_VSC_ENABLE_PROG_HEADER; > > - if (dev_priv->psr.y_cord_support) > > - chicken |= PSR2_ADD_VERTICAL_LINE_COUNT; > > + u32 chicken = PSR2_VSC_ENABLE_PROG_HEADER > > + | PSR2_ADD_VERTICAL_LINE_COUNT; > > I915_WRITE(CHICKEN_TRANS(cpu_transcoder), > > chicken); > > > > I915_WRITE(EDP_PSR_DEBUG, > > -- > > 2.16.2 > >
On Thu, Mar 15, 2018 at 06:04:27PM -0700, Souza, Jose wrote: > On Thu, 2018-03-15 at 17:35 -0700, Rodrigo Vivi wrote: > > On Wed, Mar 14, 2018 at 03:36:13PM -0700, José Roberto de Souza > > wrote: > > > Move to only one place the sink requirements that the actual driver > > > needs to enable PSR2. > > > > > > Also intel_psr2_config_valid() is called every time the crtc config > > > is computed, wasting some time every time it was checking for > > > Y coordinate requirement. > > > > > > This allow us to nuke y_cord_support and some of VSC setup code > > > that > > > was handling a scenario that would never happen(PSR2 without Y > > > coordinate). > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 1 - > > > drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++------ > > > -------------- > > > 2 files changed, 28 insertions(+), 28 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h > > > index 8a584273f897..d4bc8d18f56c 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -603,7 +603,6 @@ struct i915_psr { > > > unsigned busy_frontbuffer_bits; > > > bool psr2_support; > > > bool link_standby; > > > - bool y_cord_support; > > > bool colorimetry_support; > > > bool alpm; > > > bool has_hw_tracking; > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > > b/drivers/gpu/drm/i915/intel_psr.c > > > index 9811f5f0bc75..62d97d5576d1 100644 > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > @@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp > > > *intel_dp) > > > intel_display_power_put(dev_priv, > > > psr_aux_domain(intel_dp)); > > > } > > > > > > -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp) > > > +static bool intel_dp_get_y_coord_required(struct intel_dp > > > *intel_dp) > > > { > > > uint8_t psr_caps = 0; > > > > > > @@ -137,22 +137,38 @@ void intel_psr_init_dpcd(struct intel_dp > > > *intel_dp) > > > > > > if (INTEL_GEN(dev_priv) >= 9 && > > > (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) { > > > - uint8_t frame_sync_cap; > > > + uint8_t frame_sync_cap, y_coord_req; > > > > > > dev_priv->psr.sink_support = true; > > > + > > > + /* PSR2 needs frame sync to do selective updates > > > */ > > > if (drm_dp_dpcd_readb(&intel_dp->aux, > > > DP_SINK_DEVICE_AUX_FRAME_SYN > > > C_CAP, > > > &frame_sync_cap) != 1) > > > frame_sync_cap = 0; > > > frame_sync_cap = (frame_sync_cap & > > > DP_AUX_FRAME_SYNC_CAP); > > > - /* PSR2 needs frame sync as well */ > > > - dev_priv->psr.psr2_support = frame_sync_cap; > > > - DRM_DEBUG_KMS("PSR2 %s on sink", > > > - dev_priv->psr.psr2_support ? > > > "supported" : "not supported"); > > > + > > > + /* > > > + * FIXME: Enable PSR2 only for y-coordinate PSR2 > > > panels > > > + * After GTC implementation, remove this > > > restriction. > > > > I believe we should remove the FIXME and just comment out that non y- > > coordinate > > would need GTC that we currently don't implement. > > > > later if we change our minds and decided to add that support we > > revisit here > > but without a FIXME poluting code for something we don't plan to > > support for now. > > > > > + * > > > + * All panels that supports PSR version 3 that is > > > > PSRv3? is there really something formal about that? > > Yes, from eDP spec: > > PSR Support and Version > > 00h = Panel Self Refresh Capability is not supported (default). > If PSR/PSR2 is supported, the SET_POWER_CAPABLE bit in the > EDP_GENERAL_CAPABILITY_1 register (DPCD Address 00701h, bit 7) must > be set to 1. > 01h = Panel Self Refresh capability is supported, PSR version is 01h > (PSR). > 02h = Panel Self Refresh with Selective Update (SU) capability is > supported, > PSR version is 02h (PSR2). Y-coordinates for SU are not supported. > Supported > by eDP v1.4 (and higher). > 03h = Panel Self Refresh with SU capability and Y-coordinate supported, > PSR > version is 03h (PSR2 + Y-coordinate). Supported by eDP v1.4a (and > higher). Oh cool! now I understand the comment. Could we add the comment like in spec ie s/PSR version 3 that is PSR2 + Y-coordinate/PSR version 03h (PSR2 + Y-coordinate)/ ? just to avoid giving impression of a PSR3 support and someone freaking out thinking that we would need much more changes ;) > > > > > > + * PSR2 + Y-coordinate support can handle Y > > > coordinates in VSC > > > + * but we are only sure that it is going to be > > > used when > > > + * required by the panel. This way panel is > > > capable to do > > > + * selective update even without a valid aux frame > > > sync. > > > + */ > > > + y_coord_req = > > > intel_dp_get_y_coord_required(intel_dp); > > > + > > > + dev_priv->psr.psr2_support = frame_sync_cap && > > > y_coord_req; > > > + if (dev_priv->psr.psr2_support) > > > + DRM_DEBUG_KMS("PSR2 supported on sink\n"); > > > + else > > > + DRM_DEBUG_KMS("PSR2 not supported on sink" > > > + "(frame sync: %d Y-coord > > > required: %d)\n", > > > + frame_sync_cap, > > > y_coord_req); > > > > > > if (dev_priv->psr.psr2_support) { > > > - dev_priv->psr.y_cord_support = > > > - intel_dp_get_y_cord_status(intel_d > > > p); > > > dev_priv->psr.colorimetry_support = > > > intel_dp_get_colorimetry_status(in > > > tel_dp); > > > dev_priv->psr.alpm = > > > @@ -198,16 +214,12 @@ static void hsw_psr_setup_vsc(struct intel_dp > > > *intel_dp, > > > memset(&psr_vsc, 0, sizeof(psr_vsc)); > > > psr_vsc.sdp_header.HB0 = 0; > > > psr_vsc.sdp_header.HB1 = 0x7; > > > - if (dev_priv->psr.colorimetry_support && > > > - dev_priv->psr.y_cord_support) { > > > + if (dev_priv->psr.colorimetry_support) { > > > psr_vsc.sdp_header.HB2 = 0x5; > > > psr_vsc.sdp_header.HB3 = 0x13; > > > - } else if (dev_priv->psr.y_cord_support) { > > > + } else { > > > psr_vsc.sdp_header.HB2 = 0x4; > > > psr_vsc.sdp_header.HB3 = 0xe; > > > - } else { > > > - psr_vsc.sdp_header.HB2 = 0x3; > > > - psr_vsc.sdp_header.HB3 = 0xc; > > > } > > > } else { > > > /* Prepare VSC packet as per EDP 1.3 spec, Table > > > 3.10 */ > > > @@ -478,15 +490,6 @@ static bool intel_psr2_config_valid(struct > > > intel_dp *intel_dp, > > > return false; > > > } > > > > > > - /* > > > - * FIXME:enable psr2 only for y-cordinate psr2 panels > > > - * After gtc implementation , remove this restriction. > > > - */ > > > - if (!dev_priv->psr.y_cord_support) { > > > - DRM_DEBUG_KMS("PSR2 not enabled, panel does not > > > support Y coordinate\n"); > > > - return false; > > > - } > > > - > > > return true; > > > } > > > > > > @@ -586,14 +589,12 @@ static void hsw_psr_enable_source(struct > > > intel_dp *intel_dp, > > > struct drm_device *dev = dig_port->base.base.dev; > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > enum transcoder cpu_transcoder = crtc_state- > > > >cpu_transcoder; > > > - u32 chicken; > > > > > > psr_aux_io_power_get(intel_dp); > > > > > > if (dev_priv->psr.psr2_support) { > > > - chicken = PSR2_VSC_ENABLE_PROG_HEADER; > > > - if (dev_priv->psr.y_cord_support) > > > - chicken |= PSR2_ADD_VERTICAL_LINE_COUNT; > > > + u32 chicken = PSR2_VSC_ENABLE_PROG_HEADER > > > + | PSR2_ADD_VERTICAL_LINE_COUNT; > > > I915_WRITE(CHICKEN_TRANS(cpu_transcoder), > > > chicken); > > > > > > I915_WRITE(EDP_PSR_DEBUG, > > > -- > > > 2.16.2 > > >
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8a584273f897..d4bc8d18f56c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -603,7 +603,6 @@ struct i915_psr { unsigned busy_frontbuffer_bits; bool psr2_support; bool link_standby; - bool y_cord_support; bool colorimetry_support; bool alpm; bool has_hw_tracking; diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 9811f5f0bc75..62d97d5576d1 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp *intel_dp) intel_display_power_put(dev_priv, psr_aux_domain(intel_dp)); } -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp) +static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp) { uint8_t psr_caps = 0; @@ -137,22 +137,38 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 9 && (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) { - uint8_t frame_sync_cap; + uint8_t frame_sync_cap, y_coord_req; dev_priv->psr.sink_support = true; + + /* PSR2 needs frame sync to do selective updates */ if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, &frame_sync_cap) != 1) frame_sync_cap = 0; frame_sync_cap = (frame_sync_cap & DP_AUX_FRAME_SYNC_CAP); - /* PSR2 needs frame sync as well */ - dev_priv->psr.psr2_support = frame_sync_cap; - DRM_DEBUG_KMS("PSR2 %s on sink", - dev_priv->psr.psr2_support ? "supported" : "not supported"); + + /* + * FIXME: Enable PSR2 only for y-coordinate PSR2 panels + * After GTC implementation, remove this restriction. + * + * All panels that supports PSR version 3 that is + * PSR2 + Y-coordinate support can handle Y coordinates in VSC + * but we are only sure that it is going to be used when + * required by the panel. This way panel is capable to do + * selective update even without a valid aux frame sync. + */ + y_coord_req = intel_dp_get_y_coord_required(intel_dp); + + dev_priv->psr.psr2_support = frame_sync_cap && y_coord_req; + if (dev_priv->psr.psr2_support) + DRM_DEBUG_KMS("PSR2 supported on sink\n"); + else + DRM_DEBUG_KMS("PSR2 not supported on sink" + "(frame sync: %d Y-coord required: %d)\n", + frame_sync_cap, y_coord_req); if (dev_priv->psr.psr2_support) { - dev_priv->psr.y_cord_support = - intel_dp_get_y_cord_status(intel_dp); dev_priv->psr.colorimetry_support = intel_dp_get_colorimetry_status(intel_dp); dev_priv->psr.alpm = @@ -198,16 +214,12 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp, memset(&psr_vsc, 0, sizeof(psr_vsc)); psr_vsc.sdp_header.HB0 = 0; psr_vsc.sdp_header.HB1 = 0x7; - if (dev_priv->psr.colorimetry_support && - dev_priv->psr.y_cord_support) { + if (dev_priv->psr.colorimetry_support) { psr_vsc.sdp_header.HB2 = 0x5; psr_vsc.sdp_header.HB3 = 0x13; - } else if (dev_priv->psr.y_cord_support) { + } else { psr_vsc.sdp_header.HB2 = 0x4; psr_vsc.sdp_header.HB3 = 0xe; - } else { - psr_vsc.sdp_header.HB2 = 0x3; - psr_vsc.sdp_header.HB3 = 0xc; } } else { /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ @@ -478,15 +490,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; } - /* - * FIXME:enable psr2 only for y-cordinate psr2 panels - * After gtc implementation , remove this restriction. - */ - if (!dev_priv->psr.y_cord_support) { - DRM_DEBUG_KMS("PSR2 not enabled, panel does not support Y coordinate\n"); - return false; - } - return true; } @@ -586,14 +589,12 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp, struct drm_device *dev = dig_port->base.base.dev; struct drm_i915_private *dev_priv = to_i915(dev); enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; - u32 chicken; psr_aux_io_power_get(intel_dp); if (dev_priv->psr.psr2_support) { - chicken = PSR2_VSC_ENABLE_PROG_HEADER; - if (dev_priv->psr.y_cord_support) - chicken |= PSR2_ADD_VERTICAL_LINE_COUNT; + u32 chicken = PSR2_VSC_ENABLE_PROG_HEADER + | PSR2_ADD_VERTICAL_LINE_COUNT; I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken); I915_WRITE(EDP_PSR_DEBUG,
Move to only one place the sink requirements that the actual driver needs to enable PSR2. Also intel_psr2_config_valid() is called every time the crtc config is computed, wasting some time every time it was checking for Y coordinate requirement. This allow us to nuke y_cord_support and some of VSC setup code that was handling a scenario that would never happen(PSR2 without Y coordinate). Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++-------------------- 2 files changed, 28 insertions(+), 28 deletions(-)