Message ID | 20200825171331.17971-1-jose.souza@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v3,1/3] drm/i915/display: Compute has_drrs after compute has_psr | expand |
On 2020-08-25 at 10:13:29 -0700, José Roberto de Souza wrote: > DRRS and PSR can't be enable together, so giving preference to PSR > as it allows more power-savings by complete shutting down display, > so to guarantee this, it should compute DRRS state after compute PSR. > > Cc: Srinivas K <srinivasx.k@intel.com> > Cc: Hariom Pandey <hariom.pandey@intel.com> > Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 52 +++++++++++++++---------- > 1 file changed, 32 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 79c27f91f42c..a08d03c61b02 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2575,6 +2575,34 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp, > intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA); > } > > +static void > +intel_dp_drrs_compute_config(struct intel_dp *intel_dp, > + struct intel_crtc_state *pipe_config, > + int output_bpp, bool constant_n) > +{ > + struct intel_connector *intel_connector = intel_dp->attached_connector; > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > + > + /* > + * DRRS and PSR can't be enable together, so giving preference to PSR > + * as it allows more power-savings by complete shutting down display, > + * so to guarantee this, intel_dp_drrs_compute_config() must be called > + * after intel_psr_compute_config(). > + */ > + if (pipe_config->has_psr) > + return; > + > + if (!intel_connector->panel.downclock_mode || > + dev_priv->drrs.type != SEAMLESS_DRRS_SUPPORT) > + return; > + > + pipe_config->has_drrs = true; > + intel_link_compute_m_n(output_bpp, pipe_config->lane_count, > + intel_connector->panel.downclock_mode->clock, > + pipe_config->port_clock, &pipe_config->dp_m2_n2, > + constant_n, pipe_config->fec_enable); > +} > + > int > intel_dp_compute_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config, > @@ -2605,7 +2633,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, > if (ret) > return ret; > > - pipe_config->has_drrs = false; IMHO this assignment is required, i was thinking a case, when a crtc is attached to more than one connector, suppose first eDP connector supports DRRS from panel.downclock_mode and drrs.type but another DP connector won't support it in that case has_drrs will be still true. Please correct me if i am wrong here. Thanks, Anshuman Gupta. > if (!intel_dp_port_has_audio(dev_priv, port)) > pipe_config->has_audio = false; > else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO) > @@ -2657,21 +2684,12 @@ intel_dp_compute_config(struct intel_encoder *encoder, > &pipe_config->dp_m_n, > constant_n, pipe_config->fec_enable); > > - if (intel_connector->panel.downclock_mode != NULL && > - dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) { > - pipe_config->has_drrs = true; > - intel_link_compute_m_n(output_bpp, > - pipe_config->lane_count, > - intel_connector->panel.downclock_mode->clock, > - pipe_config->port_clock, > - &pipe_config->dp_m2_n2, > - constant_n, pipe_config->fec_enable); > - } > - > if (!HAS_DDI(dev_priv)) > intel_dp_set_clock(encoder, pipe_config); > > intel_psr_compute_config(intel_dp, pipe_config); > + intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp, > + constant_n); > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state); > > @@ -7730,16 +7748,10 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp, > { > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > - if (!crtc_state->has_drrs) { > - drm_dbg_kms(&dev_priv->drm, "Panel doesn't support DRRS\n"); > + if (!crtc_state->has_drrs) > return; > - } > > - if (dev_priv->psr.enabled) { > - drm_dbg_kms(&dev_priv->drm, > - "PSR enabled. Not enabling DRRS.\n"); > - return; > - } > + drm_dbg_kms(&dev_priv->drm, "Enabling DRRS\n"); > > mutex_lock(&dev_priv->drrs.mutex); > if (dev_priv->drrs.dp) { > -- > 2.28.0 >
On Wed, 2020-08-26 at 12:56 +0530, Anshuman Gupta wrote: > On 2020-08-25 at 10:13:29 -0700, José Roberto de Souza wrote: > > DRRS and PSR can't be enable together, so giving preference to PSR > > as it allows more power-savings by complete shutting down display, > > so to guarantee this, it should compute DRRS state after compute PSR. > > > > Cc: Srinivas K < > > srinivasx.k@intel.com > > > > > Cc: Hariom Pandey < > > hariom.pandey@intel.com > > > > > Reviewed-by: Anshuman Gupta < > > anshuman.gupta@intel.com > > > > > Signed-off-by: José Roberto de Souza < > > jose.souza@intel.com > > > > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 52 +++++++++++++++---------- > > 1 file changed, 32 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index 79c27f91f42c..a08d03c61b02 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -2575,6 +2575,34 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp, > > intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA); > > } > > > > +static void > > +intel_dp_drrs_compute_config(struct intel_dp *intel_dp, > > + struct intel_crtc_state *pipe_config, > > + int output_bpp, bool constant_n) > > +{ > > + struct intel_connector *intel_connector = intel_dp->attached_connector; > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > + > > + /* > > + * DRRS and PSR can't be enable together, so giving preference to PSR > > + * as it allows more power-savings by complete shutting down display, > > + * so to guarantee this, intel_dp_drrs_compute_config() must be called > > + * after intel_psr_compute_config(). > > + */ > > + if (pipe_config->has_psr) > > + return; > > + > > + if (!intel_connector->panel.downclock_mode || > > + dev_priv->drrs.type != SEAMLESS_DRRS_SUPPORT) > > + return; > > + > > + pipe_config->has_drrs = true; > > + intel_link_compute_m_n(output_bpp, pipe_config->lane_count, > > + intel_connector->panel.downclock_mode->clock, > > + pipe_config->port_clock, &pipe_config->dp_m2_n2, > > + constant_n, pipe_config->fec_enable); > > +} > > + > > int > > intel_dp_compute_config(struct intel_encoder *encoder, > > struct intel_crtc_state *pipe_config, > > @@ -2605,7 +2633,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > if (ret) > > return ret; > > > > - pipe_config->has_drrs = false; > > IMHO this assignment is required, i was thinking a case, when a crtc is attached to more than > one connector, suppose first eDP connector supports DRRS from panel.downclock_mode and > drrs.type but another DP connector won't support it in that case has_drrs will be still > true. > Please correct me if i am wrong here. i915 only supports one connector per pipe/CRTC, if that was the case all other flags in intel_crtc_state would also have the same behaviour as has_drrs. > Thanks, > Anshuman Gupta. > > if (!intel_dp_port_has_audio(dev_priv, port)) > > pipe_config->has_audio = false; > > else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO) > > @@ -2657,21 +2684,12 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > &pipe_config->dp_m_n, > > constant_n, pipe_config->fec_enable); > > > > - if (intel_connector->panel.downclock_mode != NULL && > > - dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) { > > - pipe_config->has_drrs = true; > > - intel_link_compute_m_n(output_bpp, > > - pipe_config->lane_count, > > - intel_connector->panel.downclock_mode->clock, > > - pipe_config->port_clock, > > - &pipe_config->dp_m2_n2, > > - constant_n, pipe_config->fec_enable); > > - } > > - > > if (!HAS_DDI(dev_priv)) > > intel_dp_set_clock(encoder, pipe_config); > > > > intel_psr_compute_config(intel_dp, pipe_config); > > + intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp, > > + constant_n); > > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); > > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state); > > > > @@ -7730,16 +7748,10 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp, > > { > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > > - if (!crtc_state->has_drrs) { > > - drm_dbg_kms(&dev_priv->drm, "Panel doesn't support DRRS\n"); > > + if (!crtc_state->has_drrs) > > return; > > - } > > > > - if (dev_priv->psr.enabled) { > > - drm_dbg_kms(&dev_priv->drm, > > - "PSR enabled. Not enabling DRRS.\n"); > > - return; > > - } > > + drm_dbg_kms(&dev_priv->drm, "Enabling DRRS\n"); > > > > mutex_lock(&dev_priv->drrs.mutex); > > if (dev_priv->drrs.dp) { > > -- > > 2.28.0 > >
On 2020-08-26 at 22:17:38 +0530, Souza, Jose wrote: > On Wed, 2020-08-26 at 12:56 +0530, Anshuman Gupta wrote: > > On 2020-08-25 at 10:13:29 -0700, José Roberto de Souza wrote: > > > DRRS and PSR can't be enable together, so giving preference to PSR > > > as it allows more power-savings by complete shutting down display, > > > so to guarantee this, it should compute DRRS state after compute PSR. > > > > > > Cc: Srinivas K < > > > srinivasx.k@intel.com > > > > > > > Cc: Hariom Pandey < > > > hariom.pandey@intel.com > > > > > > > Reviewed-by: Anshuman Gupta < > > > anshuman.gupta@intel.com > > > > > > > Signed-off-by: José Roberto de Souza < > > > jose.souza@intel.com > > > > > > > --- > > > drivers/gpu/drm/i915/display/intel_dp.c | 52 +++++++++++++++---------- > > > 1 file changed, 32 insertions(+), 20 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 79c27f91f42c..a08d03c61b02 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -2575,6 +2575,34 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp, > > > intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA); > > > } > > > > > > +static void > > > +intel_dp_drrs_compute_config(struct intel_dp *intel_dp, > > > + struct intel_crtc_state *pipe_config, > > > + int output_bpp, bool constant_n) > > > +{ > > > + struct intel_connector *intel_connector = intel_dp->attached_connector; > > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > + > > > + /* > > > + * DRRS and PSR can't be enable together, so giving preference to PSR > > > + * as it allows more power-savings by complete shutting down display, > > > + * so to guarantee this, intel_dp_drrs_compute_config() must be called > > > + * after intel_psr_compute_config(). > > > + */ > > > + if (pipe_config->has_psr) > > > + return; > > > + > > > + if (!intel_connector->panel.downclock_mode || > > > + dev_priv->drrs.type != SEAMLESS_DRRS_SUPPORT) > > > + return; > > > + > > > + pipe_config->has_drrs = true; > > > + intel_link_compute_m_n(output_bpp, pipe_config->lane_count, > > > + intel_connector->panel.downclock_mode->clock, > > > + pipe_config->port_clock, &pipe_config->dp_m2_n2, > > > + constant_n, pipe_config->fec_enable); > > > +} > > > + > > > int > > > intel_dp_compute_config(struct intel_encoder *encoder, > > > struct intel_crtc_state *pipe_config, > > > @@ -2605,7 +2633,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > > if (ret) > > > return ret; > > > > > > - pipe_config->has_drrs = false; > > > > IMHO this assignment is required, i was thinking a case, when a crtc is attached to more than > > one connector, suppose first eDP connector supports DRRS from panel.downclock_mode and > > drrs.type but another DP connector won't support it in that case has_drrs will be still > > true. > > Please correct me if i am wrong here. > > i915 only supports one connector per pipe/CRTC, if that was the case all other flags in intel_crtc_state would also have the same behaviour as > has_drrs. Actually once on kabylake i had witnessed this use case when a CRTC was attached to multiple connectors, AFAIU theortically also it seems possible as intel_modeset_pipe_config iterate for each connector in state for a given crtc i.e (connector_state->crtc == crtc) and call encoder->compute_config(). If you are sure above case can't happen for drrs you can use my RB. Applicable for [PATCH 3/3] of this series, Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com> > > > Thanks, > > Anshuman Gupta. > > > if (!intel_dp_port_has_audio(dev_priv, port)) > > > pipe_config->has_audio = false; > > > else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO) > > > @@ -2657,21 +2684,12 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > > &pipe_config->dp_m_n, > > > constant_n, pipe_config->fec_enable); > > > > > > - if (intel_connector->panel.downclock_mode != NULL && > > > - dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) { > > > - pipe_config->has_drrs = true; > > > - intel_link_compute_m_n(output_bpp, > > > - pipe_config->lane_count, > > > - intel_connector->panel.downclock_mode->clock, > > > - pipe_config->port_clock, > > > - &pipe_config->dp_m2_n2, > > > - constant_n, pipe_config->fec_enable); > > > - } > > > - > > > if (!HAS_DDI(dev_priv)) > > > intel_dp_set_clock(encoder, pipe_config); > > > > > > intel_psr_compute_config(intel_dp, pipe_config); > > > + intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp, > > > + constant_n); > > > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); > > > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state); > > > > > > @@ -7730,16 +7748,10 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp, > > > { > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > > > > - if (!crtc_state->has_drrs) { > > > - drm_dbg_kms(&dev_priv->drm, "Panel doesn't support DRRS\n"); > > > + if (!crtc_state->has_drrs) > > > return; > > > - } > > > > > > - if (dev_priv->psr.enabled) { > > > - drm_dbg_kms(&dev_priv->drm, > > > - "PSR enabled. Not enabling DRRS.\n"); > > > - return; > > > - } > > > + drm_dbg_kms(&dev_priv->drm, "Enabling DRRS\n"); > > > > > > mutex_lock(&dev_priv->drrs.mutex); > > > if (dev_priv->drrs.dp) { > > > -- > > > 2.28.0 > > >
On Thu, 2020-08-27 at 17:39 +0530, Anshuman Gupta wrote: > On 2020-08-26 at 22:17:38 +0530, Souza, Jose wrote: > > On Wed, 2020-08-26 at 12:56 +0530, Anshuman Gupta wrote: > > > On 2020-08-25 at 10:13:29 -0700, José Roberto de Souza wrote: > > > > DRRS and PSR can't be enable together, so giving preference to PSR > > > > as it allows more power-savings by complete shutting down display, > > > > so to guarantee this, it should compute DRRS state after compute PSR. > > > > > > > > Cc: Srinivas K < > > > > srinivasx.k@intel.com > > > > > > > > > > > > Cc: Hariom Pandey < > > > > hariom.pandey@intel.com > > > > > > > > > > > > Reviewed-by: Anshuman Gupta < > > > > anshuman.gupta@intel.com > > > > > > > > > > > > Signed-off-by: José Roberto de Souza < > > > > jose.souza@intel.com > > > > > > > > > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp.c | 52 +++++++++++++++---------- > > > > 1 file changed, 32 insertions(+), 20 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > > index 79c27f91f42c..a08d03c61b02 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > @@ -2575,6 +2575,34 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp, > > > > intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA); > > > > } > > > > > > > > +static void > > > > +intel_dp_drrs_compute_config(struct intel_dp *intel_dp, > > > > + struct intel_crtc_state *pipe_config, > > > > + int output_bpp, bool constant_n) > > > > +{ > > > > + struct intel_connector *intel_connector = intel_dp->attached_connector; > > > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > > + > > > > + /* > > > > + * DRRS and PSR can't be enable together, so giving preference to PSR > > > > + * as it allows more power-savings by complete shutting down display, > > > > + * so to guarantee this, intel_dp_drrs_compute_config() must be called > > > > + * after intel_psr_compute_config(). > > > > + */ > > > > + if (pipe_config->has_psr) > > > > + return; > > > > + > > > > + if (!intel_connector->panel.downclock_mode || > > > > + dev_priv->drrs.type != SEAMLESS_DRRS_SUPPORT) > > > > + return; > > > > + > > > > + pipe_config->has_drrs = true; > > > > + intel_link_compute_m_n(output_bpp, pipe_config->lane_count, > > > > + intel_connector->panel.downclock_mode->clock, > > > > + pipe_config->port_clock, &pipe_config->dp_m2_n2, > > > > + constant_n, pipe_config->fec_enable); > > > > +} > > > > + > > > > int > > > > intel_dp_compute_config(struct intel_encoder *encoder, > > > > struct intel_crtc_state *pipe_config, > > > > @@ -2605,7 +2633,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > > > if (ret) > > > > return ret; > > > > > > > > - pipe_config->has_drrs = false; > > > > > > IMHO this assignment is required, i was thinking a case, when a crtc is attached to more than > > > one connector, suppose first eDP connector supports DRRS from panel.downclock_mode and > > > drrs.type but another DP connector won't support it in that case has_drrs will be still > > > true. > > > Please correct me if i am wrong here. > > > > i915 only supports one connector per pipe/CRTC, if that was the case all other flags in intel_crtc_state would also have the same behaviour as > > has_drrs. > > Actually once on kabylake i had witnessed this use case when a CRTC was attached to multiple connectors, AFAIU theortically also > it seems possible as intel_modeset_pipe_config iterate for each connector in state for a given crtc i.e (connector_state->crtc == crtc) > and call encoder->compute_config(). > If you are sure above case can't happen for drrs you can use my RB. > Applicable for [PATCH 3/3] of this series, > Reviewed-by: Anshuman Gupta < > anshuman.gupta@intel.com PSR would have a lot of issues if such scenario was possible.Patches pushed, thanks for the review. > > > > > Thanks, > > > Anshuman Gupta. > > > > if (!intel_dp_port_has_audio(dev_priv, port)) > > > > pipe_config->has_audio = false; > > > > else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO) > > > > @@ -2657,21 +2684,12 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > > > &pipe_config->dp_m_n, > > > > constant_n, pipe_config->fec_enable); > > > > > > > > - if (intel_connector->panel.downclock_mode != NULL && > > > > - dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) { > > > > - pipe_config->has_drrs = true; > > > > - intel_link_compute_m_n(output_bpp, > > > > - pipe_config->lane_count, > > > > - intel_connector->panel.downclock_mode->clock, > > > > - pipe_config->port_clock, > > > > - &pipe_config->dp_m2_n2, > > > > - constant_n, pipe_config->fec_enable); > > > > - } > > > > - > > > > if (!HAS_DDI(dev_priv)) > > > > intel_dp_set_clock(encoder, pipe_config); > > > > > > > > intel_psr_compute_config(intel_dp, pipe_config); > > > > + intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp, > > > > + constant_n); > > > > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); > > > > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state); > > > > > > > > @@ -7730,16 +7748,10 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp, > > > > { > > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > > > > > > - if (!crtc_state->has_drrs) { > > > > - drm_dbg_kms(&dev_priv->drm, "Panel doesn't support DRRS\n"); > > > > + if (!crtc_state->has_drrs) > > > > return; > > > > - } > > > > > > > > - if (dev_priv->psr.enabled) { > > > > - drm_dbg_kms(&dev_priv->drm, > > > > - "PSR enabled. Not enabling DRRS.\n"); > > > > - return; > > > > - } > > > > + drm_dbg_kms(&dev_priv->drm, "Enabling DRRS\n"); > > > > > > > > mutex_lock(&dev_priv->drrs.mutex); > > > > if (dev_priv->drrs.dp) { > > > > -- > > > > 2.28.0 > > > >
On Tue, 2020-08-25 at 18:50 +0000, Patchwork wrote: > Patch Details > Series: series starting with [v3,1/3] drm/i915/display: Compute has_drrs after compute has_psr > URL: https://patchwork.freedesktop.org/series/80989/ > State: failure > Details: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18398/index.html > CI Bug Log - changes from CI_DRM_8924_full -> Patchwork_18398_full > Summary > FAILURE > > Serious unknown changes coming with Patchwork_18398_full absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_18398_full, please notify your bug team to allow them > to document this new failure mode, which will reduce false positives in CI. > > Possible new issues > Here are the unknown changes that may have been introduced in Patchwork_18398_full: > > IGT changes > Possible regressions > igt@gem_sync@basic-store-all: > shard-tglb: PASS -> FAIL Changes not related, so patches pushed to dinq.Thanks for the reviews Anshuman. > Known issues > Here are the changes found in Patchwork_18398_full that come from known issues: > > IGT changes > Issues hit > igt@gem_exec_reloc@basic-concurrent0: > > shard-apl: PASS -> TIMEOUT (i915#1635 / i915#1958) +1 similar issue > igt@gem_exec_whisper@basic-fds-forked-all: > > shard-kbl: PASS -> TIMEOUT (i915#1958) +1 similar issue > igt@gem_exec_whisper@basic-queues-priority-all: > > shard-glk: PASS -> TIMEOUT (i915#1958) +1 similar issue > igt@i915_pm_dc@dc5-psr: > > shard-skl: PASS -> INCOMPLETE (i915#198) > igt@kms_big_fb@x-tiled-64bpp-rotate-0: > > shard-glk: PASS -> DMESG-FAIL (i915#118 / i915#95) +1 similar issue > igt@kms_big_fb@x-tiled-8bpp-rotate-0: > > shard-apl: PASS -> DMESG-WARN (i915#1635 / i915#1982) +1 similar issue > igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic: > > shard-hsw: PASS -> FAIL (i915#96) > igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic: > > shard-kbl: PASS -> DMESG-WARN (i915#1982) > igt@kms_cursor_legacy@flip-vs-cursor-legacy: > > shard-skl: PASS -> FAIL (i915#2346) > igt@kms_flip@flip-vs-expired-vblank@c-dp1: > > shard-apl: PASS -> FAIL (i915#1635 / i915#79) > igt@kms_flip@flip-vs-suspend@c-hdmi-a1: > > shard-hsw: PASS -> INCOMPLETE (i915#2055) > igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc: > > shard-tglb: PASS -> DMESG-WARN (i915#1982) +1 similar issue > igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-gtt: > > shard-skl: PASS -> FAIL (i915#49) > igt@kms_hdr@bpc-switch-suspend: > > shard-kbl: PASS -> DMESG-WARN (i915#180) +7 similar issues > igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min: > > shard-skl: PASS -> FAIL (fdo#108145 / i915#265) +1 similar issue > igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: > > shard-skl: PASS -> DMESG-WARN (i915#1982) +7 similar issues > igt@kms_psr@psr2_sprite_plane_onoff: > > shard-iclb: PASS -> SKIP (fdo#109441) > igt@perf@blocking-parameterized: > > shard-iclb: PASS -> FAIL (i915#1542) > Possible fixes > igt@gem_exec_gttfill@all: > > shard-apl: TIMEOUT (i915#1635 / i915#1958) -> PASS > igt@gem_exec_nop@basic-sequential: > > shard-tglb: TIMEOUT (i915#1958) -> PASS +2 similar issues > igt@gem_exec_parallel@engines@basic: > > shard-kbl: INCOMPLETE -> PASS > igt@gem_exec_reloc@basic-concurrent0: > > shard-skl: TIMEOUT (i915#1958) -> PASS > igt@gem_exec_whisper@basic-contexts: > > shard-glk: TIMEOUT (i915#1958) -> PASS +3 similar issues > igt@gem_exec_whisper@basic-forked: > > shard-iclb: TIMEOUT (i915#1958) -> PASS +1 similar issue > igt@gem_exec_whisper@basic-normal: > > shard-kbl: TIMEOUT (i915#1958) -> PASS > igt@gem_sync@basic-store-all: > > shard-apl: FAIL (i915#1635) -> PASS > > shard-kbl: FAIL -> PASS > > igt@i915_selftest@mock@requests: > > shard-skl: INCOMPLETE (i915#198 / i915#2278) -> PASS > igt@kms_cursor_crc@pipe-b-cursor-128x128-sliding: > > shard-skl: DMESG-FAIL (i915#1982 / i915#54) -> PASS > igt@kms_flip@2x-wf_vblank-ts-check@ab-vga1-hdmi-a1: > > shard-hsw: DMESG-WARN (i915#1982) -> PASS > igt@kms_flip@flip-vs-suspend-interruptible@a-dp1: > > shard-kbl: DMESG-WARN (i915#180) -> PASS +6 similar issues > igt@kms_flip@plain-flip-fb-recreate-interruptible@a-edp1: > > shard-skl: FAIL (i915#2122) -> PASS > igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-pwrite: > > shard-kbl: DMESG-WARN (i915#1982) -> PASS > igt@kms_frontbuffer_tracking@fbcpsr-suspend: > > shard-tglb: DMESG-WARN (i915#1982) -> PASS +3 similar issues > igt@kms_hdr@bpc-switch-dpms: > > shard-skl: FAIL (i915#1188) -> PASS > igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping: > > shard-iclb: DMESG-WARN (i915#1982) -> PASS > igt@kms_psr@psr2_primary_mmap_cpu: > > shard-iclb: SKIP (fdo#109441) -> PASS +1 similar issue > igt@perf@gen8-unprivileged-single-ctx-counters: > > shard-skl: DMESG-WARN (i915#1982) -> PASS +10 similar issues > Warnings > igt@gem_exec_reloc@basic-many-active@rcs0: > > shard-apl: FAIL (i915#1635) -> FAIL (i915#1635 / i915#2389) > igt@i915_pm_dc@dc3co-vpb-simulation: > > shard-snb: SKIP (fdo#109271) -> INCOMPLETE (i915#82) > igt@kms_content_protection@atomic: > > shard-kbl: TIMEOUT (i915#1319) -> TIMEOUT (i915#1319 / i915#1958) > igt@kms_psr2_su@page_flip: > > shard-iclb: SKIP (i915#1911) -> SKIP (fdo#109642 / fdo#111068) > igt@perf@polling-parameterized: > > shard-skl: FAIL (i915#1542) -> DMESG-FAIL (i915#1982) > igt@runner@aborted: > > shard-skl: (FAIL, FAIL) (i915#1436 / i915#2029) -> FAIL (i915#1436) > {name}: This element is suppressed. This means it is ignored when computing > the status of the difference (SUCCESS, WARNING, or FAILURE). > > Participating hosts (10 -> 10) > No changes in participating hosts > > Build changes > Linux: CI_DRM_8924 -> Patchwork_18398 > CI-20190529: 20190529 > CI_DRM_8924: a8c0611e412aab46eab5475b0117d074892b96e2 @ git://anongit.freedesktop.org/gfx-ci/linux > IGT_5770: f1d0c240ea2e631dfb9f493f37f8fb61cb2b1cf2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > Patchwork_18398: 7d42e19c12ea5344d13aa31756cf847649e9985b @ git://anongit.freedesktop.org/gfx-ci/linux > piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit > >
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 79c27f91f42c..a08d03c61b02 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2575,6 +2575,34 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp, intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA); } +static void +intel_dp_drrs_compute_config(struct intel_dp *intel_dp, + struct intel_crtc_state *pipe_config, + int output_bpp, bool constant_n) +{ + struct intel_connector *intel_connector = intel_dp->attached_connector; + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); + + /* + * DRRS and PSR can't be enable together, so giving preference to PSR + * as it allows more power-savings by complete shutting down display, + * so to guarantee this, intel_dp_drrs_compute_config() must be called + * after intel_psr_compute_config(). + */ + if (pipe_config->has_psr) + return; + + if (!intel_connector->panel.downclock_mode || + dev_priv->drrs.type != SEAMLESS_DRRS_SUPPORT) + return; + + pipe_config->has_drrs = true; + intel_link_compute_m_n(output_bpp, pipe_config->lane_count, + intel_connector->panel.downclock_mode->clock, + pipe_config->port_clock, &pipe_config->dp_m2_n2, + constant_n, pipe_config->fec_enable); +} + int intel_dp_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, @@ -2605,7 +2633,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, if (ret) return ret; - pipe_config->has_drrs = false; if (!intel_dp_port_has_audio(dev_priv, port)) pipe_config->has_audio = false; else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO) @@ -2657,21 +2684,12 @@ intel_dp_compute_config(struct intel_encoder *encoder, &pipe_config->dp_m_n, constant_n, pipe_config->fec_enable); - if (intel_connector->panel.downclock_mode != NULL && - dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) { - pipe_config->has_drrs = true; - intel_link_compute_m_n(output_bpp, - pipe_config->lane_count, - intel_connector->panel.downclock_mode->clock, - pipe_config->port_clock, - &pipe_config->dp_m2_n2, - constant_n, pipe_config->fec_enable); - } - if (!HAS_DDI(dev_priv)) intel_dp_set_clock(encoder, pipe_config); intel_psr_compute_config(intel_dp, pipe_config); + intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp, + constant_n); intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state); @@ -7730,16 +7748,10 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp, { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); - if (!crtc_state->has_drrs) { - drm_dbg_kms(&dev_priv->drm, "Panel doesn't support DRRS\n"); + if (!crtc_state->has_drrs) return; - } - if (dev_priv->psr.enabled) { - drm_dbg_kms(&dev_priv->drm, - "PSR enabled. Not enabling DRRS.\n"); - return; - } + drm_dbg_kms(&dev_priv->drm, "Enabling DRRS\n"); mutex_lock(&dev_priv->drrs.mutex); if (dev_priv->drrs.dp) {