* [PATCH 2/2] drm/i915: Clean up DSC vs. not bpp handling
2019-03-26 14:49 [PATCH 1/2] drm/i915: Do not enable FEC without DSC Ville Syrjala
@ 2019-03-26 14:49 ` Ville Syrjala
2019-03-26 16:02 ` Manasi Navare
2019-03-26 16:00 ` [PATCH 1/2] drm/i915: Do not enable FEC without DSC Manasi Navare
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjala @ 2019-03-26 14:49 UTC (permalink / raw)
To: intel-gfx; +Cc: Manasi Navare
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
No point in duplicating all this code when we can just
use a variable top hold the output bpp (the only thing
that differs between the two branches).
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.comk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bbf678561509..b26007a32318 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2126,7 +2126,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
to_intel_digital_connector_state(conn_state);
bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
DP_DPCD_QUIRK_CONSTANT_N);
- int ret;
+ int ret, output_bpp;
if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
pipe_config->has_pch_encoder = true;
@@ -2190,25 +2190,22 @@ intel_dp_compute_config(struct intel_encoder *encoder,
intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
}
- if (!pipe_config->dsc_params.compression_enable)
- intel_link_compute_m_n(pipe_config->pipe_bpp,
- pipe_config->lane_count,
- adjusted_mode->crtc_clock,
- pipe_config->port_clock,
- &pipe_config->dp_m_n,
- constant_n);
+ if (pipe_config->dsc_params.compression_enable)
+ output_bpp = pipe_config->dsc_params.compressed_bpp;
else
- intel_link_compute_m_n(pipe_config->dsc_params.compressed_bpp,
- pipe_config->lane_count,
- adjusted_mode->crtc_clock,
- pipe_config->port_clock,
- &pipe_config->dp_m_n,
- constant_n);
+ output_bpp = pipe_config->pipe_bpp;
+
+ intel_link_compute_m_n(output_bpp,
+ pipe_config->lane_count,
+ adjusted_mode->crtc_clock,
+ pipe_config->port_clock,
+ &pipe_config->dp_m_n,
+ constant_n);
if (intel_connector->panel.downclock_mode != NULL &&
dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
pipe_config->has_drrs = true;
- intel_link_compute_m_n(pipe_config->pipe_bpp,
+ intel_link_compute_m_n(output_bpp,
pipe_config->lane_count,
intel_connector->panel.downclock_mode->clock,
pipe_config->port_clock,
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] drm/i915: Clean up DSC vs. not bpp handling
2019-03-26 14:49 ` [PATCH 2/2] drm/i915: Clean up DSC vs. not bpp handling Ville Syrjala
@ 2019-03-26 16:02 ` Manasi Navare
2019-04-11 18:27 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Manasi Navare @ 2019-03-26 16:02 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, Manasi Navare
On Tue, Mar 26, 2019 at 04:49:03PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> No point in duplicating all this code when we can just
> use a variable top hold the output bpp (the only thing
> that differs between the two branches).
>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.comk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
This clean up looks good, thank you for catching this and cleaning it up.
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/intel_dp.c | 27 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index bbf678561509..b26007a32318 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2126,7 +2126,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> to_intel_digital_connector_state(conn_state);
> bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> DP_DPCD_QUIRK_CONSTANT_N);
> - int ret;
> + int ret, output_bpp;
>
> if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
> pipe_config->has_pch_encoder = true;
> @@ -2190,25 +2190,22 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
> }
>
> - if (!pipe_config->dsc_params.compression_enable)
> - intel_link_compute_m_n(pipe_config->pipe_bpp,
> - pipe_config->lane_count,
> - adjusted_mode->crtc_clock,
> - pipe_config->port_clock,
> - &pipe_config->dp_m_n,
> - constant_n);
> + if (pipe_config->dsc_params.compression_enable)
> + output_bpp = pipe_config->dsc_params.compressed_bpp;
> else
> - intel_link_compute_m_n(pipe_config->dsc_params.compressed_bpp,
> - pipe_config->lane_count,
> - adjusted_mode->crtc_clock,
> - pipe_config->port_clock,
> - &pipe_config->dp_m_n,
> - constant_n);
> + output_bpp = pipe_config->pipe_bpp;
> +
> + intel_link_compute_m_n(output_bpp,
> + pipe_config->lane_count,
> + adjusted_mode->crtc_clock,
> + pipe_config->port_clock,
> + &pipe_config->dp_m_n,
> + constant_n);
>
> if (intel_connector->panel.downclock_mode != NULL &&
> dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> pipe_config->has_drrs = true;
> - intel_link_compute_m_n(pipe_config->pipe_bpp,
> + intel_link_compute_m_n(output_bpp,
> pipe_config->lane_count,
> intel_connector->panel.downclock_mode->clock,
> pipe_config->port_clock,
> --
> 2.19.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] drm/i915: Clean up DSC vs. not bpp handling
2019-03-26 16:02 ` Manasi Navare
@ 2019-04-11 18:27 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-04-11 18:27 UTC (permalink / raw)
Cc: intel-gfx, Manasi Navare
On Tue, Mar 26, 2019 at 09:02:36AM -0700, Manasi Navare wrote:
> On Tue, Mar 26, 2019 at 04:49:03PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > No point in duplicating all this code when we can just
> > use a variable top hold the output bpp (the only thing
> > that differs between the two branches).
> >
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.comk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> This clean up looks good, thank you for catching this and cleaning it up.
>
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Patch 2 pushed. Thanks for the review. Patch 1 could still use a rubber
stamp...
>
> Manasi
>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 27 ++++++++++++---------------
> > 1 file changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index bbf678561509..b26007a32318 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2126,7 +2126,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > to_intel_digital_connector_state(conn_state);
> > bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> > DP_DPCD_QUIRK_CONSTANT_N);
> > - int ret;
> > + int ret, output_bpp;
> >
> > if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
> > pipe_config->has_pch_encoder = true;
> > @@ -2190,25 +2190,22 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
> > }
> >
> > - if (!pipe_config->dsc_params.compression_enable)
> > - intel_link_compute_m_n(pipe_config->pipe_bpp,
> > - pipe_config->lane_count,
> > - adjusted_mode->crtc_clock,
> > - pipe_config->port_clock,
> > - &pipe_config->dp_m_n,
> > - constant_n);
> > + if (pipe_config->dsc_params.compression_enable)
> > + output_bpp = pipe_config->dsc_params.compressed_bpp;
> > else
> > - intel_link_compute_m_n(pipe_config->dsc_params.compressed_bpp,
> > - pipe_config->lane_count,
> > - adjusted_mode->crtc_clock,
> > - pipe_config->port_clock,
> > - &pipe_config->dp_m_n,
> > - constant_n);
> > + output_bpp = pipe_config->pipe_bpp;
> > +
> > + intel_link_compute_m_n(output_bpp,
> > + pipe_config->lane_count,
> > + adjusted_mode->crtc_clock,
> > + pipe_config->port_clock,
> > + &pipe_config->dp_m_n,
> > + constant_n);
> >
> > if (intel_connector->panel.downclock_mode != NULL &&
> > dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> > pipe_config->has_drrs = true;
> > - intel_link_compute_m_n(pipe_config->pipe_bpp,
> > + intel_link_compute_m_n(output_bpp,
> > pipe_config->lane_count,
> > intel_connector->panel.downclock_mode->clock,
> > pipe_config->port_clock,
> > --
> > 2.19.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC
2019-03-26 14:49 [PATCH 1/2] drm/i915: Do not enable FEC without DSC Ville Syrjala
2019-03-26 14:49 ` [PATCH 2/2] drm/i915: Clean up DSC vs. not bpp handling Ville Syrjala
@ 2019-03-26 16:00 ` Manasi Navare
2019-03-26 16:16 ` Ville Syrjälä
2019-03-26 19:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Manasi Navare @ 2019-03-26 16:00 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, Manasi Navare
On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we enable FEC even when DSC is no used. While that is
> theoretically valid supposedly there isn't much of a benefit from
> this. But more importantly we do not account for the FEC link
> bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> So the code may think we have enough bandwidth when we in fact
> do not.
>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.comk>
> Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 326de12c3f44..bbf678561509 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> int pipe_bpp;
> int ret;
>
> + pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> + intel_dp_supports_fec(intel_dp, pipe_config);
> +
We could still not enable DSC after this point since it has more checks in this
function. Even though in that case we would fail the encoder config so wouldnt
matter if we have enabled FEC or not, but its less intutive.
IMHO, the ideal place to set the fec enable is in intel_dp_compute_link_config()
after the all to dsc_compute_config and set it only if pipe_config->dsc_params.compression_enable
Manasi
> if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> return -EINVAL;
>
> @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> return -EINVAL;
>
> - pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> - intel_dp_supports_fec(intel_dp, pipe_config);
> -
> ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
> if (ret < 0)
> return ret;
> --
> 2.19.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC
2019-03-26 16:00 ` [PATCH 1/2] drm/i915: Do not enable FEC without DSC Manasi Navare
@ 2019-03-26 16:16 ` Ville Syrjälä
2019-03-26 18:10 ` Manasi Navare
0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2019-03-26 16:16 UTC (permalink / raw)
Cc: intel-gfx, Manasi Navare
On Tue, Mar 26, 2019 at 09:00:27AM -0700, Manasi Navare wrote:
> On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently we enable FEC even when DSC is no used. While that is
> > theoretically valid supposedly there isn't much of a benefit from
> > this. But more importantly we do not account for the FEC link
> > bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> > So the code may think we have enough bandwidth when we in fact
> > do not.
> >
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.comk>
> > Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 326de12c3f44..bbf678561509 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> > int pipe_bpp;
> > int ret;
> >
> > + pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > + intel_dp_supports_fec(intel_dp, pipe_config);
> > +
>
> We could still not enable DSC after this point since it has more checks in this
> function. Even though in that case we would fail the encoder config so wouldnt
> matter if we have enabled FEC or not, but its less intutive.
> IMHO, the ideal place to set the fec enable is in intel_dp_compute_link_config()
> after the all to dsc_compute_config and set it only if pipe_config->dsc_params.compression_enable
That would require changing intel_dp_supports_dsc() which I decided
wasn't worth the hassle.
>
> Manasi
>
> > if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> > return -EINVAL;
> >
> > @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> > return -EINVAL;
> >
> > - pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > - intel_dp_supports_fec(intel_dp, pipe_config);
> > -
> > ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
> > if (ret < 0)
> > return ret;
> > --
> > 2.19.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC
2019-03-26 16:16 ` Ville Syrjälä
@ 2019-03-26 18:10 ` Manasi Navare
2019-03-27 13:13 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Manasi Navare @ 2019-03-26 18:10 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Manasi Navare
On Tue, Mar 26, 2019 at 06:16:57PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 26, 2019 at 09:00:27AM -0700, Manasi Navare wrote:
> > On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Currently we enable FEC even when DSC is no used. While that is
> > > theoretically valid supposedly there isn't much of a benefit from
> > > this. But more importantly we do not account for the FEC link
> > > bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> > > So the code may think we have enough bandwidth when we in fact
> > > do not.
> > >
> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.comk>
> > > Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 326de12c3f44..bbf678561509 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> > > int pipe_bpp;
> > > int ret;
> > >
> > > + pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > > + intel_dp_supports_fec(intel_dp, pipe_config);
> > > +
> >
> > We could still not enable DSC after this point since it has more checks in this
> > function. Even though in that case we would fail the encoder config so wouldnt
> > matter if we have enabled FEC or not, but its less intutive.
> > IMHO, the ideal place to set the fec enable is in intel_dp_compute_link_config()
> > after the all to dsc_compute_config and set it only if pipe_config->dsc_params.compression_enable
>
> That would require changing intel_dp_supports_dsc() which I decided
> wasn't worth the hassle.
Hmm, yea because intel_dp_supports_dsc depends on fec_enable.
In that case we would need to change that to use intel_dp_supports_fec(). TBH that makes more sense since
we should set dp_supports based on HW FEC capability and then set the crtc_state->fec_enable based
on crtc_state->dsc_params.compression_enable. But since it doesnt change the functionality, I am ok
either ways.
Manasi
>
> >
> > Manasi
> >
> > > if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> > > return -EINVAL;
> > >
> > > @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> > > return -EINVAL;
> > >
> > > - pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > > - intel_dp_supports_fec(intel_dp, pipe_config);
> > > -
> > > ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
> > > if (ret < 0)
> > > return ret;
> > > --
> > > 2.19.2
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC
2019-03-26 18:10 ` Manasi Navare
@ 2019-03-27 13:13 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-03-27 13:13 UTC (permalink / raw)
Cc: intel-gfx, Manasi Navare
On Tue, Mar 26, 2019 at 11:10:44AM -0700, Manasi Navare wrote:
> On Tue, Mar 26, 2019 at 06:16:57PM +0200, Ville Syrjälä wrote:
> > On Tue, Mar 26, 2019 at 09:00:27AM -0700, Manasi Navare wrote:
> > > On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >
> > > > Currently we enable FEC even when DSC is no used. While that is
> > > > theoretically valid supposedly there isn't much of a benefit from
> > > > this. But more importantly we do not account for the FEC link
> > > > bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> > > > So the code may think we have enough bandwidth when we in fact
> > > > do not.
> > > >
> > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > > Cc: Manasi Navare <manasi.d.navare@intel.comk>
> > > > Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 326de12c3f44..bbf678561509 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> > > > int pipe_bpp;
> > > > int ret;
> > > >
> > > > + pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > > > + intel_dp_supports_fec(intel_dp, pipe_config);
> > > > +
> > >
> > > We could still not enable DSC after this point since it has more checks in this
> > > function. Even though in that case we would fail the encoder config so wouldnt
> > > matter if we have enabled FEC or not, but its less intutive.
> > > IMHO, the ideal place to set the fec enable is in intel_dp_compute_link_config()
> > > after the all to dsc_compute_config and set it only if pipe_config->dsc_params.compression_enable
> >
> > That would require changing intel_dp_supports_dsc() which I decided
> > wasn't worth the hassle.
>
> Hmm, yea because intel_dp_supports_dsc depends on fec_enable.
> In that case we would need to change that to use intel_dp_supports_fec(). TBH that makes more sense since
> we should set dp_supports based on HW FEC capability and then set the crtc_state->fec_enable based
> on crtc_state->dsc_params.compression_enable. But since it doesnt change the functionality, I am ok
> either ways.
So r-b?
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Do not enable FEC without DSC
2019-03-26 14:49 [PATCH 1/2] drm/i915: Do not enable FEC without DSC Ville Syrjala
2019-03-26 14:49 ` [PATCH 2/2] drm/i915: Clean up DSC vs. not bpp handling Ville Syrjala
2019-03-26 16:00 ` [PATCH 1/2] drm/i915: Do not enable FEC without DSC Manasi Navare
@ 2019-03-26 19:20 ` Patchwork
2019-03-27 4:02 ` ✓ Fi.CI.IGT: " Patchwork
2019-04-11 19:11 ` [PATCH 1/2] " Manasi Navare
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-03-26 19:20 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Do not enable FEC without DSC
URL : https://patchwork.freedesktop.org/series/58588/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5818 -> Patchwork_12602
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/58588/revisions/1/mbox/
Known issues
------------
Here are the changes found in Patchwork_12602 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_module_load@reload:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: PASS -> FAIL [fdo#108511]
* igt@i915_selftest@live_uncore:
- fi-ivb-3770: PASS -> DMESG-FAIL [fdo#110210]
* igt@kms_frontbuffer_tracking@basic:
- fi-byt-clapper: PASS -> FAIL [fdo#103167]
#### Possible fixes ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-hsw-4770: SKIP [fdo#109271] -> PASS +2
#### Warnings ####
* igt@i915_selftest@live_contexts:
- fi-icl-u3: INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569]
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
Participating hosts (45 -> 38)
------------------------------
Missing (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-gdg-551
Build changes
-------------
* Linux: CI_DRM_5818 -> Patchwork_12602
CI_DRM_5818: de0e80842f3d103996e99cfe27f999690c2ee06e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4905: a350b9f9f606296b1599c3617c8530a8985709e2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12602: d4765b6682ad9caeb34c3bab5eb6e0d3eeb4e483 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
d4765b6682ad drm/i915: Clean up DSC vs. not bpp handling
ebed6de93f4d drm/i915: Do not enable FEC without DSC
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12602/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Do not enable FEC without DSC
2019-03-26 14:49 [PATCH 1/2] drm/i915: Do not enable FEC without DSC Ville Syrjala
` (2 preceding siblings ...)
2019-03-26 19:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
@ 2019-03-27 4:02 ` Patchwork
2019-04-11 19:11 ` [PATCH 1/2] " Manasi Navare
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-03-27 4:02 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Do not enable FEC without DSC
URL : https://patchwork.freedesktop.org/series/58588/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5818_full -> Patchwork_12602_full
====================================================
Summary
-------
**WARNING**
Minor unknown changes coming with Patchwork_12602_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12602_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_12602_full:
### IGT changes ###
#### Warnings ####
* igt@kms_setmode@basic:
- shard-iclb: FAIL [fdo#99912] -> INCOMPLETE
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* {igt@gem_exec_big@single}:
- shard-kbl: PASS -> FAIL
Known issues
------------
Here are the changes found in Patchwork_12602_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_parallel@bsd2-fds:
- shard-iclb: NOTRUN -> SKIP [fdo#109276] +6
* igt@gem_mocs_settings@mocs-rc6-dirty-render:
- shard-iclb: NOTRUN -> SKIP [fdo#110206]
* igt@gem_ppgtt@blt-vs-render-ctxn:
- shard-iclb: PASS -> INCOMPLETE [fdo#109801]
* igt@gem_pread@pagefault-pread:
- shard-iclb: NOTRUN -> SKIP [fdo#109277]
* igt@gem_workarounds@suspend-resume-context:
- shard-skl: PASS -> INCOMPLETE [fdo#104108] / [fdo#107773] +1
* igt@i915_pm_rpm@gem-execbuf-stress-pc8:
- shard-iclb: NOTRUN -> SKIP [fdo#109506]
* igt@i915_pm_rpm@modeset-lpsp-stress-no-wait:
- shard-kbl: NOTRUN -> SKIP [fdo#109271] +22
* igt@i915_pm_rpm@modeset-non-lpsp-stress:
- shard-iclb: NOTRUN -> SKIP [fdo#109308]
* igt@i915_pm_rpm@system-suspend-execbuf:
- shard-skl: PASS -> INCOMPLETE [fdo#104108] / [fdo#107807]
* igt@i915_pm_rps@min-max-config-loaded:
- shard-iclb: NOTRUN -> FAIL [fdo#108059]
* igt@kms_busy@extended-modeset-hang-newfb-render-a:
- shard-skl: NOTRUN -> DMESG-WARN [fdo#110222] +4
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-f:
- shard-kbl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2
* igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
- shard-iclb: PASS -> DMESG-WARN [fdo#110222]
* igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-d:
- shard-iclb: NOTRUN -> SKIP [fdo#109278] +4
* igt@kms_chamelium@hdmi-cmp-planes-random:
- shard-iclb: NOTRUN -> SKIP [fdo#109284] +2
* igt@kms_cursor_legacy@2x-cursor-vs-flip-legacy:
- shard-iclb: NOTRUN -> SKIP [fdo#109274] +3
* igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
- shard-iclb: NOTRUN -> FAIL [fdo#103355] +1
* igt@kms_cursor_legacy@cursor-vs-flip-legacy:
- shard-iclb: PASS -> FAIL [fdo#103355]
* igt@kms_fbcon_fbt@psr:
- shard-iclb: NOTRUN -> FAIL [fdo#103833]
* igt@kms_fbcon_fbt@psr-suspend:
- shard-skl: NOTRUN -> FAIL [fdo#103833]
* igt@kms_flip@flip-vs-expired-vblank:
- shard-skl: NOTRUN -> FAIL [fdo#105363]
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-glk: PASS -> FAIL [fdo#102887] / [fdo#105363]
* igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
- shard-skl: PASS -> FAIL [fdo#103167]
- shard-iclb: PASS -> FAIL [fdo#103167] +1
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
- shard-skl: PASS -> FAIL [fdo#105682]
- shard-kbl: PASS -> FAIL [fdo#105682] +1
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-cpu:
- shard-snb: NOTRUN -> SKIP [fdo#109271] +71
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
- shard-iclb: PASS -> FAIL [fdo#109247] +10
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
- shard-iclb: NOTRUN -> FAIL [fdo#109247] +3
* igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc:
- shard-iclb: NOTRUN -> SKIP [fdo#109280] +19
* igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-cpu:
- shard-iclb: PASS -> FAIL [fdo#105682] / [fdo#109247]
* igt@kms_panel_fitting@legacy:
- shard-skl: NOTRUN -> FAIL [fdo#105456]
* igt@kms_pipe_b_c_ivb@from-pipe-c-to-b-with-3-lanes:
- shard-iclb: NOTRUN -> SKIP [fdo#109289] +2
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- shard-iclb: PASS -> FAIL [fdo#103375]
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-f:
- shard-skl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +17
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
- shard-kbl: NOTRUN -> FAIL [fdo#108145]
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-kbl: PASS -> FAIL [fdo#108145]
- shard-skl: PASS -> FAIL [fdo#108145]
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
- shard-skl: NOTRUN -> FAIL [fdo#108145] +1
* igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
- shard-kbl: NOTRUN -> FAIL [fdo#108145] / [fdo#108590]
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-snb: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +9
* igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format:
- shard-glk: PASS -> SKIP [fdo#109271] / [fdo#109278]
* igt@kms_psr2_su@frontbuffer:
- shard-iclb: PASS -> SKIP [fdo#109642]
* igt@kms_psr@primary_mmap_cpu:
- shard-iclb: PASS -> FAIL [fdo#107383] / [fdo#110215] +2
* igt@kms_psr@primary_render:
- shard-iclb: NOTRUN -> FAIL [fdo#107383] / [fdo#110215]
* igt@kms_psr@psr2_basic:
- shard-iclb: PASS -> SKIP [fdo#109441] +1
* igt@kms_setmode@basic:
- shard-snb: NOTRUN -> FAIL [fdo#99912]
* igt@kms_vblank@pipe-b-ts-continuation-modeset-hang:
- shard-apl: PASS -> FAIL [fdo#104894] +1
* igt@perf_pmu@busy-accuracy-50-vcs1:
- shard-skl: NOTRUN -> SKIP [fdo#109271] +170
* igt@prime_nv_api@nv_self_import_to_different_fd:
- shard-iclb: NOTRUN -> SKIP [fdo#109291] +2
#### Possible fixes ####
* igt@gem_ctx_isolation@vecs0-s3:
- shard-skl: INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS
* {igt@gem_exec_big@single}:
- shard-glk: FAIL -> PASS
* igt@gem_tiled_swapping@non-threaded:
- shard-iclb: DMESG-WARN [fdo#108686] -> PASS
* igt@i915_pm_rpm@modeset-lpsp:
- shard-skl: INCOMPLETE [fdo#107807] -> PASS +1
* igt@kms_cursor_crc@cursor-64x64-suspend:
- shard-skl: INCOMPLETE [fdo#104108] -> PASS
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-skl: FAIL [fdo#105363] -> PASS
* igt@kms_flip@flip-vs-suspend-interruptible:
- shard-kbl: INCOMPLETE [fdo#103665] -> PASS
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
- shard-iclb: FAIL [fdo#109247] -> PASS +17
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
- shard-iclb: FAIL [fdo#103167] -> PASS +6
* {igt@kms_plane@pixel-format-pipe-a-planes-source-clamping}:
- shard-glk: SKIP [fdo#109271] -> PASS
* igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
- shard-glk: SKIP [fdo#109271] / [fdo#109278] -> PASS
* igt@kms_psr@psr2_primary_page_flip:
- shard-iclb: SKIP [fdo#109441] -> PASS +2
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-kbl: FAIL [fdo#109016] -> PASS
* igt@kms_vblank@pipe-c-ts-continuation-suspend:
- shard-iclb: FAIL [fdo#104894] -> PASS
#### Warnings ####
* igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
- shard-skl: INCOMPLETE [fdo#107807] -> SKIP [fdo#109271]
* igt@i915_selftest@live_contexts:
- shard-iclb: INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569]
* igt@kms_rotation_crc@multiplane-rotation:
- shard-kbl: INCOMPLETE [fdo#103665] -> FAIL [fdo#109016]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
[fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#103833]: https://bugs.freedesktop.org/show_bug.cgi?id=103833
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
[fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
[fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
[fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108059]: https://bugs.freedesktop.org/show_bug.cgi?id=108059
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
[fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
[fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
[fdo#110206]: https://bugs.freedesktop.org/show_bug.cgi?id=110206
[fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
[fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 9)
------------------------------
Missing (1): shard-hsw
Build changes
-------------
* Linux: CI_DRM_5818 -> Patchwork_12602
CI_DRM_5818: de0e80842f3d103996e99cfe27f999690c2ee06e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4905: a350b9f9f606296b1599c3617c8530a8985709e2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12602: d4765b6682ad9caeb34c3bab5eb6e0d3eeb4e483 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12602/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC
2019-03-26 14:49 [PATCH 1/2] drm/i915: Do not enable FEC without DSC Ville Syrjala
` (3 preceding siblings ...)
2019-03-27 4:02 ` ✓ Fi.CI.IGT: " Patchwork
@ 2019-04-11 19:11 ` Manasi Navare
2019-04-11 20:49 ` Ville Syrjälä
4 siblings, 1 reply; 12+ messages in thread
From: Manasi Navare @ 2019-04-11 19:11 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, Manasi Navare
On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we enable FEC even when DSC is no used. While that is
> theoretically valid supposedly there isn't much of a benefit from
> this. But more importantly we do not account for the FEC link
> bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> So the code may think we have enough bandwidth when we in fact
> do not.
>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.comk>
There is a typo in the email address:
manasi.d.navare@intel.com
> Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Makes sense to me
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 326de12c3f44..bbf678561509 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> int pipe_bpp;
> int ret;
>
> + pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> + intel_dp_supports_fec(intel_dp, pipe_config);
> +
> if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> return -EINVAL;
>
> @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> return -EINVAL;
>
> - pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> - intel_dp_supports_fec(intel_dp, pipe_config);
> -
> ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
> if (ret < 0)
> return ret;
> --
> 2.19.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC
2019-04-11 19:11 ` [PATCH 1/2] " Manasi Navare
@ 2019-04-11 20:49 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-04-11 20:49 UTC (permalink / raw)
Cc: intel-gfx, Manasi Navare
On Thu, Apr 11, 2019 at 12:11:42PM -0700, Manasi Navare wrote:
> On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently we enable FEC even when DSC is no used. While that is
> > theoretically valid supposedly there isn't much of a benefit from
> > this. But more importantly we do not account for the FEC link
> > bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> > So the code may think we have enough bandwidth when we in fact
> > do not.
> >
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.comk>
>
> There is a typo in the email address:
> manasi.d.navare@intel.com
Thanks for the reminder. I already forgot about this and was about to
push without fixing it. Sadly it seems I had copy pasted it from the
same place to both of these patches, so one got pushed with the bad
address.
Hmm. Looks like this escaped into 5.0 already. Added cc:stable
and pushed to dinq. Thanks for the review.
>
> > Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Makes sense to me
>
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
>
> Manasi
>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 326de12c3f44..bbf678561509 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> > int pipe_bpp;
> > int ret;
> >
> > + pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > + intel_dp_supports_fec(intel_dp, pipe_config);
> > +
> > if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> > return -EINVAL;
> >
> > @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> > return -EINVAL;
> >
> > - pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > - intel_dp_supports_fec(intel_dp, pipe_config);
> > -
> > ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
> > if (ret < 0)
> > return ret;
> > --
> > 2.19.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread