On Fri, Jan 15, 2016 at 02:55:26PM +0200, Jyri Sarha wrote: > Only set atomic connector callbacks if the tda998x driver is bound to > a drm driver that supports atomic modesetting. At least > drm_crtc_helper_set_config() calls connectors dpms callback without > knowing that it may assume atomic modeset support. Calling > drm_atomic_helper_connector_dpms() causes a crash with a driver that > does not have atomic state initialized. Hi Jyri, > > Fixes commit 9736e988d328 ("drm/i2c: tda998x: Add support for atomic > modesetting"). First, sorry for the breakage, I did not had any other board to test the change on. > > Signed-off-by: Jyri Sarha <jsarha@xxxxxx> > --- > I am not sure if this is the best way to fix the issue, but after > 9736e988d328 Beaglebone-Black HDMI is totally broken and this fixes > the issue. I am currently working on atomic modesetting for tilcdc, > but there is no way it makes it to 4.5 release. Yes, I'm not a big fan of the change either. I would've thought that you only need to update the .dpms pointer, all other hooks added are specific to atomic operations (right?). Would it not be simpler to have a tda998x_connector_dpms() function that calls the appropriate .dpms function based on the feature check? Best regards, Liviu > > Best regards, > Jyri > > drivers/gpu/drm/i2c/tda998x_drv.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c > index 012d36d..67d1408 100644 > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > @@ -1382,7 +1382,7 @@ static void tda998x_connector_destroy(struct drm_connector *connector) > drm_connector_cleanup(connector); > } > > -static const struct drm_connector_funcs tda998x_connector_funcs = { > +static const struct drm_connector_funcs tda998x_connector_atomic_funcs = { > .dpms = drm_atomic_helper_connector_dpms, > .reset = drm_atomic_helper_connector_reset, > .fill_modes = drm_helper_probe_single_connector_modes, > @@ -1392,15 +1392,28 @@ static const struct drm_connector_funcs tda998x_connector_funcs = { > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > +static const struct drm_connector_funcs tda998x_connector_funcs = { > + .dpms = drm_helper_connector_dpms, > + .fill_modes = drm_helper_probe_single_connector_modes, > + .detect = tda998x_connector_detect, > + .destroy = tda998x_connector_destroy, > +}; > + > static int tda998x_bind(struct device *dev, struct device *master, void *data) > { > struct tda998x_encoder_params *params = dev->platform_data; > struct i2c_client *client = to_i2c_client(dev); > struct drm_device *drm = data; > struct tda998x_priv *priv; > + const struct drm_connector_funcs *connector_funcs; > u32 crtcs = 0; > int ret; > > + if (drm_core_check_feature(drm, DRIVER_ATOMIC)) > + connector_funcs = &tda998x_connector_atomic_funcs; > + else > + connector_funcs = &tda998x_connector_funcs; > + > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > if (!priv) > return -ENOMEM; > @@ -1437,7 +1450,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) > drm_connector_helper_add(&priv->connector, > &tda998x_connector_helper_funcs); > ret = drm_connector_init(drm, &priv->connector, > - &tda998x_connector_funcs, > + connector_funcs, > DRM_MODE_CONNECTOR_HDMIA); > if (ret) > goto err_connector; > -- > 1.9.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html