* [PATCH RFC] drm/i2c: tda998x: Only set atomic funcs if bound to atomic drm driver
@ 2016-01-15 12:55 Jyri Sarha
2016-01-15 14:09 ` Liviu.Dudau
0 siblings, 1 reply; 3+ messages in thread
From: Jyri Sarha @ 2016-01-15 12:55 UTC (permalink / raw)
To: dri-devel, airlied, linux-omap
Cc: rmk+kernel, tomi.valkeinen, Liviu.Dudau, Jyri Sarha
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;
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH RFC] drm/i2c: tda998x: Only set atomic funcs if bound to atomic drm driver
2016-01-15 12:55 [PATCH RFC] drm/i2c: tda998x: Only set atomic funcs if bound to atomic drm driver Jyri Sarha
@ 2016-01-15 14:09 ` Liviu.Dudau
2016-01-16 19:55 ` Jyri Sarha
0 siblings, 1 reply; 3+ messages in thread
From: Liviu.Dudau @ 2016-01-15 14:09 UTC (permalink / raw)
To: Jyri Sarha; +Cc: rmk+kernel, tomi.valkeinen, linux-omap, dri-devel
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);
> if (ret)
> goto err_connector;
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH RFC] drm/i2c: tda998x: Only set atomic funcs if bound to atomic drm driver
2016-01-15 14:09 ` Liviu.Dudau
@ 2016-01-16 19:55 ` Jyri Sarha
0 siblings, 0 replies; 3+ messages in thread
From: Jyri Sarha @ 2016-01-16 19:55 UTC (permalink / raw)
To: Liviu.Dudau; +Cc: rmk+kernel, tomi.valkeinen, linux-omap, 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);
>> if (ret)
>> goto err_connector;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-01-16 19:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-15 12:55 [PATCH RFC] drm/i2c: tda998x: Only set atomic funcs if bound to atomic drm driver Jyri Sarha
2016-01-15 14:09 ` Liviu.Dudau
2016-01-16 19:55 ` Jyri Sarha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).