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.
Fixes commit 9736e988d328 ("drm/i2c: tda998x: Add support for atomic modesetting").
Signed-off-by: Jyri Sarha jsarha@ti.com --- 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.
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;
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@ti.com
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);-- 1.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 01/15/16 16:09, Liviu.Dudau@arm.com wrote:
On Fri, Jan 15, 2016 at 02:55:26PM +0200, Jyri Sarha wrote:
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?
Oh yes, that sound like a better idea, but si and appears to work too. I'll send another patch soon. Thanks.
Best regards, Jyri
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,make[4]: *** [drivers/g
+};
- 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; make[4]: *** [drivers/g
- 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);-- 1.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Jyri Sarha -
Liviu.Dudau＠arm.com