Re: [PATCH 06/10] drm/i915: Setup dummy atomic state for connectors (v3)

[Ander Conselvan de Oliveira <conselvan2@xxxxxxxxx>]

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@xxxxxxxxx>

On 01/23/2015 02:50 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.
>
> Once we start creating connector states, the DRM core will want to be
> able to clean them up for us.  We also need to hook up the destruction
> entrypoint to the core's helper.
>
> v2: Squash in the patch to set the state destruction hook (Ander & Bob)
>
> v3: Only create dummy connector states when we're actually faking
>      atomic support.  (Ander)
>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/intel_crt.c     |  2 ++
>   drivers/gpu/drm/i915/intel_display.c | 32 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_dp.c      |  2 ++
>   drivers/gpu/drm/i915/intel_dp_mst.c  |  2 ++
>   drivers/gpu/drm/i915/intel_dsi.c     |  2 ++
>   drivers/gpu/drm/i915/intel_dvo.c     |  2 ++
>   drivers/gpu/drm/i915/intel_hdmi.c    |  2 ++
>   drivers/gpu/drm/i915/intel_lvds.c    |  2 ++
>   drivers/gpu/drm/i915/intel_sdvo.c    |  2 ++
>   drivers/gpu/drm/i915/intel_tv.c      |  2 ++
>   10 files changed, 50 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index bb55368..18ee41e 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -28,6 +28,7 @@
>   #include <linux/i2c.h>
>   #include <linux/slab.h>
>   #include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc.h>
>   #include <drm/drm_crtc_helper.h>
>   #include <drm/drm_edid.h>
> @@ -792,6 +793,7 @@ static const struct drm_connector_funcs intel_crt_connector_funcs = {
>   	.fill_modes = drm_helper_probe_single_connector_modes,
>   	.destroy = intel_crt_destroy,
>   	.set_property = intel_crt_set_property,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>
>   static const struct drm_connector_helper_funcs intel_crt_connector_helper_funcs = {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 75b7ca1..b461f90 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12439,6 +12439,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);
> @@ -12569,6 +12570,37 @@ 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.
> +	 */
> +	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);
> +			}
> +		}
> +	}
> +
>   	intel_psr_init(dev);
>
>   	for_each_intel_encoder(dev, encoder) {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b38d737..1b1917b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -31,6 +31,7 @@
>   #include <linux/notifier.h>
>   #include <linux/reboot.h>
>   #include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc.h>
>   #include <drm/drm_crtc_helper.h>
>   #include <drm/drm_edid.h>
> @@ -4402,6 +4403,7 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
>   	.fill_modes = drm_helper_probe_single_connector_modes,
>   	.set_property = intel_dp_set_property,
>   	.destroy = intel_dp_connector_destroy,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>
>   static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 0091a84..f86da0f 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -26,6 +26,7 @@
>   #include <drm/drmP.h>
>   #include "i915_drv.h"
>   #include "intel_drv.h"
> +#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc_helper.h>
>   #include <drm/drm_edid.h>
>
> @@ -314,6 +315,7 @@ static const struct drm_connector_funcs intel_dp_mst_connector_funcs = {
>   	.fill_modes = drm_helper_probe_single_connector_modes,
>   	.set_property = intel_dp_mst_set_property,
>   	.destroy = intel_dp_mst_connector_destroy,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>
>   static int intel_dp_mst_get_modes(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 6620124..e9226ac 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -24,6 +24,7 @@
>    */
>
>   #include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc.h>
>   #include <drm/drm_edid.h>
>   #include <drm/i915_drm.h>
> @@ -791,6 +792,7 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
>   	.detect = intel_dsi_detect,
>   	.destroy = intel_dsi_destroy,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>
>   void intel_dsi_init(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 706ab99..1cf2e352 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -27,6 +27,7 @@
>   #include <linux/i2c.h>
>   #include <linux/slab.h>
>   #include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc.h>
>   #include "intel_drv.h"
>   #include <drm/i915_drm.h>
> @@ -390,6 +391,7 @@ static const struct drm_connector_funcs intel_dvo_connector_funcs = {
>   	.detect = intel_dvo_detect,
>   	.destroy = intel_dvo_destroy,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>
>   static const struct drm_connector_helper_funcs intel_dvo_connector_helper_funcs = {
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 200a0e7..b8fab8c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -31,6 +31,7 @@
>   #include <linux/delay.h>
>   #include <linux/hdmi.h>
>   #include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc.h>
>   #include <drm/drm_edid.h>
>   #include "intel_drv.h"
> @@ -1615,6 +1616,7 @@ static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
>   	.fill_modes = drm_helper_probe_single_connector_modes,
>   	.set_property = intel_hdmi_set_property,
>   	.destroy = intel_hdmi_destroy,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>
>   static const struct drm_connector_helper_funcs intel_hdmi_connector_helper_funcs = {
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index c7c6414..908bd42 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -32,6 +32,7 @@
>   #include <linux/i2c.h>
>   #include <linux/slab.h>
>   #include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc.h>
>   #include <drm/drm_edid.h>
>   #include "intel_drv.h"
> @@ -532,6 +533,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
>   	.fill_modes = drm_helper_probe_single_connector_modes,
>   	.set_property = intel_lvds_set_property,
>   	.destroy = intel_lvds_destroy,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>
>   static const struct drm_encoder_funcs intel_lvds_enc_funcs = {
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 5b8275b..ae00bf9 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -30,6 +30,7 @@
>   #include <linux/delay.h>
>   #include <linux/export.h>
>   #include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc.h>
>   #include <drm/drm_edid.h>
>   #include "intel_drv.h"
> @@ -2191,6 +2192,7 @@ static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
>   	.fill_modes = drm_helper_probe_single_connector_modes,
>   	.set_property = intel_sdvo_set_property,
>   	.destroy = intel_sdvo_destroy,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>
>   static const struct drm_connector_helper_funcs intel_sdvo_connector_helper_funcs = {
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 10e7ebd..d450054 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -31,6 +31,7 @@
>    */
>
>   #include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc.h>
>   #include <drm/drm_edid.h>
>   #include "intel_drv.h"
> @@ -1513,6 +1514,7 @@ static const struct drm_connector_funcs intel_tv_connector_funcs = {
>   	.destroy = intel_tv_destroy,
>   	.set_property = intel_tv_set_property,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>
>   static const struct drm_connector_helper_funcs intel_tv_connector_helper_funcs = {

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx