On Thu, 2015-03-19 at 00:38 +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 07/19] drm/i915: Copy the staged connector config to the
> > legacy atomic state
> >
> > With this in place, we can start converting pieces of the modeset code to look at
> > the connector atomic state instead of the staged config.
> >
> > v2: Handle the load detect staged config changes too. (Ander)
> > Remove unnecessary blank line. (Daniel)
> >
> > Signed-off-by: Ander Conselvan de Oliveira
> > <ander.conselvan.de.oliveira@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 52
> > +++++++++++++++++++++++++++++++-----
> > 1 file changed, 45 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index abdbd0c..6465f6d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8805,6 +8805,7 @@ bool intel_get_load_detect_pipe(struct
> > drm_connector *connector,
> > struct drm_framebuffer *fb;
> > struct drm_mode_config *config = &dev->mode_config;
> > struct drm_atomic_state *state = NULL;
> > + struct drm_connector_state *connector_state;
> > int ret, i = -1;
> >
> > DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", @@
> > -8892,6 +8893,15 @@ retry:
> >
> > state->acquire_ctx = ctx;
> >
> > + connector_state = drm_atomic_get_connector_state(state, connector);
> > + if (IS_ERR(connector_state)) {
> > + ret = PTR_ERR(connector_state);
> > + goto fail;
> > + }
> > +
> > + connector_state->crtc = crtc;
> > + connector_state->best_encoder = &intel_encoder->base;
> > +
> > if (!mode)
> > mode = &load_detect_mode;
> >
> > @@ -8957,6 +8967,7 @@ void intel_release_load_detect_pipe(struct
> > drm_connector *connector,
> > struct drm_crtc *crtc = encoder->crtc;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > struct drm_atomic_state *state;
> > + struct drm_connector_state *connector_state;
> >
> > DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
> > connector->base.id, connector->name, @@ -8964,17
> > +8975,23 @@ void intel_release_load_detect_pipe(struct drm_connector
> > *connector,
> >
> > if (old->load_detect_temp) {
> > state = drm_atomic_state_alloc(dev);
> > - if (!state) {
> > - DRM_DEBUG_KMS("can't release load detect pipe\n");
> > - return;
> > - }
> > + if (!state)
> > + goto fail;
>
> Is the above deletion of lines from code added by another patch in this series or
> from a different series? May be you can squash them into one.
That was added in patch 3, the one that adds the drm_atomic_state
parameter to intel_set_mode(). I chose to do it that way in order to not
complicate that patch unnecessarily. It wasn't possible to convert each
different call to intel_set_mode() separately, since that would create a
state of breakage that prevents bisecting.
So the approach I found most reasonable was to just add the state
parameter in patch number 3, and leave the proper population of the
atomic state to a separate patch. The main idea was to simplify the
review process.
>
> >
> > state->acquire_ctx = ctx;
> >
> > + connector_state = drm_atomic_get_connector_state(state,
> > connector);
> > + if (IS_ERR(connector_state))
> > + goto fail;
> > +
> > to_intel_connector(connector)->new_encoder = NULL;
> > intel_encoder->new_crtc = NULL;
> > intel_crtc->new_enabled = false;
> > intel_crtc->new_config = NULL;
> > +
> > + connector_state->best_encoder = NULL;
> > + connector_state->crtc = NULL;
> > +
> > intel_set_mode(crtc, NULL, 0, 0, NULL, state);
> >
> > drm_atomic_state_free(state);
> > @@ -8990,6 +9007,11 @@ void intel_release_load_detect_pipe(struct
> > drm_connector *connector,
> > /* Switch crtc and encoder back off if necessary */
> > if (old->dpms_mode != DRM_MODE_DPMS_ON)
> > connector->funcs->dpms(connector, old->dpms_mode);
> > +
> > + return;
> > +fail:
> > + DRM_DEBUG_KMS("Couldn't release load detect pipe.\n");
> > + drm_atomic_state_free(state);
> > }
>
> Learning while reviewing connector/encoder side of handling.
> But I think someone also should look at the encoder/connector side or
> atomic handling.
I think Daniel already did a high level review of this. Perhaps he could
do it again for v2 of this patch.
Ander
> >
> > static int i9xx_pll_refclk(struct drm_device *dev, @@ -11646,9 +11668,11 @@
> > intel_set_config_compute_mode_changes(struct drm_mode_set *set, static int
> > intel_modeset_stage_output_state(struct drm_device *dev,
> > struct drm_mode_set *set,
> > - struct intel_set_config *config)
> > + struct intel_set_config *config,
> > + struct drm_atomic_state *state)
> > {
> > struct intel_connector *connector;
> > + struct drm_connector_state *connector_state;
> > struct intel_encoder *encoder;
> > struct intel_crtc *crtc;
> > int ro;
> > @@ -11712,6 +11736,14 @@ intel_modeset_stage_output_state(struct
> > drm_device *dev,
> > }
> > connector->new_encoder->new_crtc = to_intel_crtc(new_crtc);
> >
> > + connector_state =
> > + drm_atomic_get_connector_state(state, &connector-
> > >base);
> > + if (IS_ERR(connector_state))
> > + return PTR_ERR(connector_state);
> > +
> > + connector_state->crtc = new_crtc;
> > + connector_state->best_encoder = &connector->new_encoder-
> > >base;
> > +
> > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d]\n",
> > connector->base.base.id,
> > connector->base.name,
> > @@ -11744,9 +11776,15 @@ intel_modeset_stage_output_state(struct
> > drm_device *dev,
> > }
> > /* Now we've also updated encoder->new_crtc for all encoders. */
> > for_each_intel_connector(dev, connector) {
> > - if (connector->new_encoder)
> > + connector_state =
> > + drm_atomic_get_connector_state(state, &connector-
> > >base);
> > +
> > + if (connector->new_encoder) {
> > if (connector->new_encoder != connector->encoder)
> > connector->encoder = connector-
> > >new_encoder;
> > + } else {
> > + connector_state->crtc = NULL;
> > + }
> > }
> > for_each_intel_crtc(dev, crtc) {
> > crtc->new_enabled = false;
> > @@ -11855,7 +11893,7 @@ static int intel_crtc_set_config(struct
> > drm_mode_set *set)
> >
> > state->acquire_ctx = dev->mode_config.acquire_ctx;
> >
> > - ret = intel_modeset_stage_output_state(dev, set, config);
> > + ret = intel_modeset_stage_output_state(dev, set, config, state);
> > if (ret)
> > goto fail;
> >
> > --
> > 2.1.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
