Re: [Intel-gfx] [PATCH 10/14] drm/i915: Populate possible_crtcs correctly
On Mon, 2018-06-25 at 14:10 +0300, Ville Syrjälä wrote:
> On Thu, Jun 21, 2018 at 06:26:04PM -0700, Dhinakaran Pandiyan wrote:
> >
> > On Fri, 2018-06-15 at 19:49 +0300, Ville Syrjala wrote:
> > >
> > > From: Ville Syrjälä
> > >
> > > Don't advertize non-exisiting crtcs in the encoder possible_crtcs
> > > bitmask.
> > >
> > How do we end up advertising non-existing CRTCs? encoder->crtc_mask
> > seems to be populated in the encoder init functions based on
> > possible
> > pipes. Do you mean pipe and crtc->index can potentially differ?
> No. Just that we may sometimes set BIT(PIPE_C) in crtc_mask when
> there are only two pipes on the device.
>
Got it, thanks.
Since crtc_mask seems to be initialized based on the platform, we
should probably warn if we find out a bit is set incorrectly.
With or without that, this patch is
Reviewed-by: Dhinakaran Pandiyan
> >
> >
> >
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 17 -
> > > 1 file changed, 16 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index b095899d68a9..3fa9da714403 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -13959,6 +13959,20 @@ static int intel_encoder_clones(struct
> > > intel_encoder *encoder)
> > > return index_mask;
> > > }
> > >
> > > +static int intel_encoder_crtcs(struct intel_encoder *encoder)
> > > +{
> > > + struct drm_device *dev = encoder->base.dev;
> > > + struct intel_crtc *crtc;
> > > + int index_mask = 0;
> > > +
> > > + for_each_intel_crtc(dev, crtc) {
> > > + if (encoder->crtc_mask & BIT(crtc->pipe))
> > > + index_mask |= drm_crtc_mask(
> > > >base);
> > > + }
> > > +
> > > + return index_mask;
> > > +}
> > > +
> > > static bool has_edp_a(struct drm_i915_private *dev_priv)
> > > {
> > > if (!IS_MOBILE(dev_priv))
> > > @@ -14211,7 +14225,8 @@ static void intel_setup_outputs(struct
> > > drm_i915_private *dev_priv)
> > > intel_psr_init(dev_priv);
> > >
> > > for_each_intel_encoder(_priv->drm, encoder) {
> > > - encoder->base.possible_crtcs = encoder-
> > > >crtc_mask;
> > > + encoder->base.possible_crtcs =
> > > + intel_encoder_crtcs(encoder);
> > > encoder->base.possible_clones =
> > > intel_encoder_clones(encoder);
> > > }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/14] drm/i915: Populate possible_crtcs correctly
On Thu, Jun 21, 2018 at 06:26:04PM -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-06-15 at 19:49 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > Don't advertize non-exisiting crtcs in the encoder possible_crtcs
> > bitmask.
> >
> How do we end up advertising non-existing CRTCs? encoder->crtc_mask
> seems to be populated in the encoder init functions based on possible
> pipes. Do you mean pipe and crtc->index can potentially differ?
No. Just that we may sometimes set BIT(PIPE_C) in crtc_mask when
there are only two pipes on the device.
>
>
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 17 -
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index b095899d68a9..3fa9da714403 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13959,6 +13959,20 @@ static int intel_encoder_clones(struct
> > intel_encoder *encoder)
> > return index_mask;
> > }
> >
> > +static int intel_encoder_crtcs(struct intel_encoder *encoder)
> > +{
> > + struct drm_device *dev = encoder->base.dev;
> > + struct intel_crtc *crtc;
> > + int index_mask = 0;
> > +
> > + for_each_intel_crtc(dev, crtc) {
> > + if (encoder->crtc_mask & BIT(crtc->pipe))
> > + index_mask |= drm_crtc_mask(>base);
> > + }
> > +
> > + return index_mask;
> > +}
> > +
> > static bool has_edp_a(struct drm_i915_private *dev_priv)
> > {
> > if (!IS_MOBILE(dev_priv))
> > @@ -14211,7 +14225,8 @@ static void intel_setup_outputs(struct
> > drm_i915_private *dev_priv)
> > intel_psr_init(dev_priv);
> >
> > for_each_intel_encoder(_priv->drm, encoder) {
> > - encoder->base.possible_crtcs = encoder->crtc_mask;
> > + encoder->base.possible_crtcs =
> > + intel_encoder_crtcs(encoder);
> > encoder->base.possible_clones =
> > intel_encoder_clones(encoder);
> > }
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/14] drm/i915: Populate possible_crtcs correctly
On Fri, 2018-06-15 at 19:49 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Don't advertize non-exisiting crtcs in the encoder possible_crtcs
> bitmask.
>
How do we end up advertising non-existing CRTCs? encoder->crtc_mask
seems to be populated in the encoder init functions based on possible
pipes. Do you mean pipe and crtc->index can potentially differ?
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 17 -
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b095899d68a9..3fa9da714403 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13959,6 +13959,20 @@ static int intel_encoder_clones(struct
> intel_encoder *encoder)
> return index_mask;
> }
>
> +static int intel_encoder_crtcs(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> + struct intel_crtc *crtc;
> + int index_mask = 0;
> +
> + for_each_intel_crtc(dev, crtc) {
> + if (encoder->crtc_mask & BIT(crtc->pipe))
> + index_mask |= drm_crtc_mask(>base);
> + }
> +
> + return index_mask;
> +}
> +
> static bool has_edp_a(struct drm_i915_private *dev_priv)
> {
> if (!IS_MOBILE(dev_priv))
> @@ -14211,7 +14225,8 @@ static void intel_setup_outputs(struct
> drm_i915_private *dev_priv)
> intel_psr_init(dev_priv);
>
> for_each_intel_encoder(_priv->drm, encoder) {
> - encoder->base.possible_crtcs = encoder->crtc_mask;
> + encoder->base.possible_crtcs =
> + intel_encoder_crtcs(encoder);
> encoder->base.possible_clones =
> intel_encoder_clones(encoder);
> }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/14] drm/i915: Populate possible_crtcs correctly
From: Ville Syrjälä
Don't advertize non-exisiting crtcs in the encoder possible_crtcs
bitmask.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b095899d68a9..3fa9da714403 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13959,6 +13959,20 @@ static int intel_encoder_clones(struct intel_encoder
*encoder)
return index_mask;
}
+static int intel_encoder_crtcs(struct intel_encoder *encoder)
+{
+ struct drm_device *dev = encoder->base.dev;
+ struct intel_crtc *crtc;
+ int index_mask = 0;
+
+ for_each_intel_crtc(dev, crtc) {
+ if (encoder->crtc_mask & BIT(crtc->pipe))
+ index_mask |= drm_crtc_mask(>base);
+ }
+
+ return index_mask;
+}
+
static bool has_edp_a(struct drm_i915_private *dev_priv)
{
if (!IS_MOBILE(dev_priv))
@@ -14211,7 +14225,8 @@ static void intel_setup_outputs(struct drm_i915_private
*dev_priv)
intel_psr_init(dev_priv);
for_each_intel_encoder(_priv->drm, encoder) {
- encoder->base.possible_crtcs = encoder->crtc_mask;
+ encoder->base.possible_crtcs =
+ intel_encoder_crtcs(encoder);
encoder->base.possible_clones =
intel_encoder_clones(encoder);
}
--
2.16.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
