| Message ID | 1421726266-31941-10-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/20/2015 05:57 AM, Matt Roper wrote: > We want to enable/test plane updates via the atomic interface, but as > soon as we flip DRIVER_ATOMIC on, the DRM core will take some atomic > codepaths to lookup properties during drmModeGetConnector() and some of > those codepaths unconditionally dereference connector->state > (specifically when looking up the CRTC ID property in > drm_atomic_connector_get_property()). Create a dummy connector state > for each connector at init time to ensure the DRM core doesn't try to > dereference a NULL connector->state. The actual connector properties > will never be updated or contain useful information, but since we're > doing this specifically for testing/debug of the plane operations (and > only when a specific kernel module option is given), that shouldn't > really matter. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 0e18879..82defd3 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12368,6 +12368,7 @@ static void intel_setup_outputs(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_encoder *encoder; > + struct drm_connector *connector; > bool dpd_is_edp = false; > > intel_lvds_init(dev); > @@ -12498,6 +12499,33 @@ static void intel_setup_outputs(struct drm_device *dev) > if (SUPPORTS_TV(dev)) > intel_tv_init(dev); > > + /* > + * FIXME: We don't have full atomic support yet, but we want to be > + * able to enable/test plane updates via the atomic interface in the > + * meantime. However as soon as we flip DRIVER_ATOMIC on, the DRM core > + * will take some atomic codepaths to lookup properties during > + * drmModeGetConnector() that unconditionally dereference > + * connector->state. > + * > + * We create a dummy connector state here for each connector to ensure > + * the DRM core doesn't try to dereference a NULL connector->state. > + * The actual connector properties will never be updated or contain > + * useful information, but since we're doing this specifically for > + * testing/debug of the plane operations (and only when a specific > + * kernel module option is given), that shouldn't really matter. > + * > + * Once atomic support for crtc's + connectors lands, this loop should > + * be removed since we'll be setting up real connector state, which > + * will contain Intel-specific properties. > + */ > + list_for_each_entry(connector, > + &dev->mode_config.connector_list, > + head) { > + if (!WARN_ON(connector->state)) > + connector->state = kzalloc(sizeof(*connector->state), > + GFP_KERNEL); > + } > + I think this will trigger a WARN in drm_connector_cleanup() on driver unload, and we also leak the dummy state. Also, I'm not sure if this changes the behaviour of drm_mode_getconnector(), since now the call to drm_connector_get_encoder() will always return NULL. Ander > intel_psr_init(dev); > > for_each_intel_encoder(dev, encoder) { >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0e18879..82defd3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12368,6 +12368,7 @@ static void intel_setup_outputs(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_encoder *encoder; + struct drm_connector *connector; bool dpd_is_edp = false; intel_lvds_init(dev); @@ -12498,6 +12499,33 @@ static void intel_setup_outputs(struct drm_device *dev) if (SUPPORTS_TV(dev)) intel_tv_init(dev); + /* + * FIXME: We don't have full atomic support yet, but we want to be + * able to enable/test plane updates via the atomic interface in the + * meantime. However as soon as we flip DRIVER_ATOMIC on, the DRM core + * will take some atomic codepaths to lookup properties during + * drmModeGetConnector() that unconditionally dereference + * connector->state. + * + * We create a dummy connector state here for each connector to ensure + * the DRM core doesn't try to dereference a NULL connector->state. + * The actual connector properties will never be updated or contain + * useful information, but since we're doing this specifically for + * testing/debug of the plane operations (and only when a specific + * kernel module option is given), that shouldn't really matter. + * + * Once atomic support for crtc's + connectors lands, this loop should + * be removed since we'll be setting up real connector state, which + * will contain Intel-specific properties. + */ + list_for_each_entry(connector, + &dev->mode_config.connector_list, + head) { + if (!WARN_ON(connector->state)) + connector->state = kzalloc(sizeof(*connector->state), + GFP_KERNEL); + } + intel_psr_init(dev); for_each_intel_encoder(dev, encoder) {
We want to enable/test plane updates via the atomic interface, but as soon as we flip DRIVER_ATOMIC on, the DRM core will take some atomic codepaths to lookup properties during drmModeGetConnector() and some of those codepaths unconditionally dereference connector->state (specifically when looking up the CRTC ID property in drm_atomic_connector_get_property()). Create a dummy connector state for each connector at init time to ensure the DRM core doesn't try to dereference a NULL connector->state. The actual connector properties will never be updated or contain useful information, but since we're doing this specifically for testing/debug of the plane operations (and only when a specific kernel module option is given), that shouldn't really matter. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)