| Message ID | 20161219082423.27798-4-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 19-12-16 om 09:24 schreef Daniel Vetter: > Drive-by fixup while looking at all the connector_list walkers - > holding connection_mutex does actually _not_ give you locking to look > at the legacy drm_connector->encoder->crtc pointer chain. That one is > solely owned by the atomic commit workers. Instead we must inspect the > atomic state. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 0b0d7e8be630..438d27f93aca 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15427,15 +15427,14 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe) > > enum pipe intel_get_pipe_from_connector(struct intel_connector *connector) > { > - struct drm_encoder *encoder = connector->base.encoder; > struct drm_device *dev = connector->base.dev; > > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); > > - if (!encoder || WARN_ON(!encoder->crtc)) > + if (!connector->base.state->crtc) > return INVALID_PIPE; > > - return to_intel_crtc(encoder->crtc)->pipe; > + return to_intel_crtc(connector->base.state->crtc)->pipe; > } > > int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data, This patch clashes with the previous patch. intel_panel_set_backlight_acpi is called from asle_set_backlight. You should probably keep connection_mutex alive for it. ~Maarten
On Tue, Dec 20, 2016 at 02:35:40PM +0100, Maarten Lankhorst wrote: > Op 19-12-16 om 09:24 schreef Daniel Vetter: > > Drive-by fixup while looking at all the connector_list walkers - > > holding connection_mutex does actually _not_ give you locking to look > > at the legacy drm_connector->encoder->crtc pointer chain. That one is > > solely owned by the atomic commit workers. Instead we must inspect the > > atomic state. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 0b0d7e8be630..438d27f93aca 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -15427,15 +15427,14 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe) > > > > enum pipe intel_get_pipe_from_connector(struct intel_connector *connector) > > { > > - struct drm_encoder *encoder = connector->base.encoder; > > struct drm_device *dev = connector->base.dev; > > > > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); > > > > - if (!encoder || WARN_ON(!encoder->crtc)) > > + if (!connector->base.state->crtc) > > return INVALID_PIPE; > > > > - return to_intel_crtc(encoder->crtc)->pipe; > > + return to_intel_crtc(connector->base.state->crtc)->pipe; > > } > > > > int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data, > > This patch clashes with the previous patch. intel_panel_set_backlight_acpi is called > from asle_set_backlight. You should probably keep connection_mutex alive for it. Oh, I've forgotten to update the commit message for the previous patch - I ended up _not_ nuking the locking because it did blow up here ;-) r-b on both if I fix up the previous commit message that we can't nuke the locking because the asle_set_backlight thing needs it for the backlight callbacks? -Daniel
Op 20-12-16 om 14:49 schreef Daniel Vetter: > On Tue, Dec 20, 2016 at 02:35:40PM +0100, Maarten Lankhorst wrote: >> Op 19-12-16 om 09:24 schreef Daniel Vetter: >>> Drive-by fixup while looking at all the connector_list walkers - >>> holding connection_mutex does actually _not_ give you locking to look >>> at the legacy drm_connector->encoder->crtc pointer chain. That one is >>> solely owned by the atomic commit workers. Instead we must inspect the >>> atomic state. >>> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_display.c | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >>> index 0b0d7e8be630..438d27f93aca 100644 >>> --- a/drivers/gpu/drm/i915/intel_display.c >>> +++ b/drivers/gpu/drm/i915/intel_display.c >>> @@ -15427,15 +15427,14 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe) >>> >>> enum pipe intel_get_pipe_from_connector(struct intel_connector *connector) >>> { >>> - struct drm_encoder *encoder = connector->base.encoder; >>> struct drm_device *dev = connector->base.dev; >>> >>> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); >>> >>> - if (!encoder || WARN_ON(!encoder->crtc)) >>> + if (!connector->base.state->crtc) >>> return INVALID_PIPE; >>> >>> - return to_intel_crtc(encoder->crtc)->pipe; >>> + return to_intel_crtc(connector->base.state->crtc)->pipe; >>> } >>> >>> int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data, >> This patch clashes with the previous patch. intel_panel_set_backlight_acpi is called >> from asle_set_backlight. You should probably keep connection_mutex alive for it. > Oh, I've forgotten to update the commit message for the previous patch - I > ended up _not_ nuking the locking because it did blow up here ;-) > > r-b on both if I fix up the previous commit message that we can't nuke the > locking because the asle_set_backlight thing needs it for the backlight > callbacks? > -Daniel Yeah ok. I'd rather we pass the state, but this will do for now. Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0b0d7e8be630..438d27f93aca 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15427,15 +15427,14 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe) enum pipe intel_get_pipe_from_connector(struct intel_connector *connector) { - struct drm_encoder *encoder = connector->base.encoder; struct drm_device *dev = connector->base.dev; WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); - if (!encoder || WARN_ON(!encoder->crtc)) + if (!connector->base.state->crtc) return INVALID_PIPE; - return to_intel_crtc(encoder->crtc)->pipe; + return to_intel_crtc(connector->base.state->crtc)->pipe; } int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
Drive-by fixup while looking at all the connector_list walkers - holding connection_mutex does actually _not_ give you locking to look at the legacy drm_connector->encoder->crtc pointer chain. That one is solely owned by the atomic commit workers. Instead we must inspect the atomic state. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)