On Thu, 2015-03-19 at 20:55 +0000, Konduru, Chandra wrote:
>
> > -----Original Message-----
> > From: Conselvan De Oliveira, Ander
> > Sent: Friday, March 13, 2015 2:49 AM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: Konduru, Chandra; Conselvan De Oliveira, Ander
> > Subject: [PATCH 05/19] drm/i915: Update dummy connector atomic state with
> > current config
> >
> > Keep that state updated so that we can write code that depends on it on the
> > follow up patches.
> >
> > v2: Fix BUG() due to stale connector_state->crtc value. (Chandra)
> > Signed-off-by: Ander Conselvan de Oliveira
> > <ander.conselvan.de.oliveira@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++++++++----
> > ----
> > 1 file changed, 32 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 62b9021..abdbd0c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10114,6 +10114,27 @@ static void
> > intel_modeset_update_staged_output_state(struct drm_device *dev)
> > }
> > }
> >
> > +/* Transitional helper to copy current connector/encoder state to
> > + * connector->state. This is needed so that code that is partially
> > + * converted to atomic does the right thing.
> > + */
> > +static void intel_modeset_update_connector_atomic_state(struct
> > +drm_device *dev) {
> > + struct intel_connector *connector;
> > +
> > + for_each_intel_connector(dev, connector) {
> > + if (connector->base.encoder) {
> > + connector->base.state->best_encoder =
> > + connector->base.encoder;
> > + connector->base.state->crtc =
> > + connector->base.encoder->crtc;
> > + } else {
> > + connector->base.state->best_encoder = NULL;
> > + connector->base.state->crtc = NULL;
> > + }
> > + }
> > +}
> > +
> > /**
> > * intel_modeset_commit_output_state
> > *
> > @@ -10137,6 +10158,8 @@ static void
> > intel_modeset_commit_output_state(struct drm_device *dev)
> > crtc->base.state->enable = crtc->new_enabled;
> > crtc->base.enabled = crtc->new_enabled;
> > }
> > +
> > + intel_modeset_update_connector_atomic_state(dev);
> > }
> >
> > static void
> > @@ -12876,15 +12899,13 @@ static void intel_setup_outputs(struct
> > drm_device *dev)
> > * be removed since we'll be setting up real connector state, which
> > * will contain Intel-specific properties.
> > */
> > - if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> > - list_for_each_entry(connector,
> > - &dev->mode_config.connector_list,
> > - head) {
> > - if (!WARN_ON(connector->state)) {
> > - connector->state =
> > - kzalloc(sizeof(*connector->state),
> > - GFP_KERNEL);
> > - }
> > + /* FIXME: need to update the comment above. */
>
> Can you fix the comment instead of adding another FIXME to fix it later?
Oh, I added that FIXME as a note to myself and then let it slip through.
I'll fix and resend.
>
> > + list_for_each_entry(connector,
> > + &dev->mode_config.connector_list,
> > + head) {
> > + if (!WARN_ON(connector->state)) {
>
> In intel_modeset_stage_output_state() there is a call to setup
> connector state:
> connector_state = drm_atomic_get_connector_state(state, &connector->base);
> Is that call is covering modeset via crtc_set_config() but not during init flow, so
> doing it here?
> If that is the case, we probably know that connector->state
> is never being set, so why have WARN_ON()?
The purpose of that WARN_ON is to remind us to remove this whole loop
when we implement connector_states with connector properties. Matt added
this here to avoid a NULL pointer dereference when using nuclear page
flip, but it should go away and be part of connector initialization.
Ander
> As lower level compute code is already
> > + connector->state = kzalloc(sizeof(*connector->state),
> > + GFP_KERNEL);
> > }
> > }
> >
> > @@ -13937,6 +13958,8 @@ void intel_modeset_setup_hw_state(struct
> > drm_device *dev,
> > "[setup_hw_state]");
> > }
> >
> > + intel_modeset_update_connector_atomic_state(dev);
> > +
> > for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> > struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
> >
> > --
> > 2.1.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
