Hi Laurent,
On 2017-11-09 11:45, Laurent Pinchart wrote:
> Hi Stefan,
>
> Thank you for the patch.
>
> On Thursday, 9 November 2017 11:14:36 EET Stefan Agner wrote:
>
> I notice you have changed the subject line. I'm not sure if the new wording is
> better, as this patch doesn't set DPMS to off, it instead skips setting it
> manually. How about "drm/fsl-dcu: Don't set connector DPMS property manually"
> or "drm/fsl-dcu: Don't set DPMS property before initializing connector" ?
In a first version I also added:
connector->dpms = DRM_MODE_DPMS_OFF;
before calling drm_connector_init, but I think this is not really
required so removed it again, but forgot to adjust the subject.
Will reset to "drm/fsl-dcu: Don't set connector DPMS property"
>
> Additionally, would you mind retaining the authorship of the patch I have
> originally submitted ?
Yeah, sorry, will send v2 with proper authorship and subject.
--
Stefan
>> Since commit 4a97a3da420b ("drm: Don't update property values for atomic
>> drivers") atomic drivers must not update property values as properties
>> are read from the state instead. To catch remaining users, the
>> drm_object_property_set_value() function now throws a warning when
>> called by atomic drivers on non-immutable properties, and we hit that
>> warning when creating connectors.
>>
>> The easy fix is to just remove the drm_object_property_set_value() as it
>> is used here to set the initial value of the connector's DPMS property
>> to OFF. The DPMS property applies on top of the connector's state crtc
>> pointer (initialized to NULL) that is the main connector on/off control,
>> and should thus default to ON.
>>
>> Fixes: 4a97a3da420b ("drm: Don't update property values for atomic drivers")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Stefan Agner <stefan@xxxxxxxx>
>> ---
>> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 5 -----
>> 1 file changed, 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c index
>> edd7d8127d19..c54806d08dd7 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
>> @@ -102,7 +102,6 @@ static int fsl_dcu_attach_panel(struct
>> fsl_dcu_drm_device *fsl_dev, {
>> struct drm_encoder *encoder = &fsl_dev->encoder;
>> struct drm_connector *connector = &fsl_dev->connector.base;
>> - struct drm_mode_config *mode_config = &fsl_dev->drm->mode_config;
>
> Oops, I had missed that, sorry.
>
>> int ret;
>>
>> fsl_dev->connector.encoder = encoder;
>> @@ -122,10 +121,6 @@ static int fsl_dcu_attach_panel(struct
>> fsl_dcu_drm_device *fsl_dev, if (ret < 0)
>> goto err_sysfs;
>>
>> - drm_object_property_set_value(&connector->base,
>> - mode_config->dpms_property,
>> - DRM_MODE_DPMS_OFF);
>> -
>> ret = drm_panel_attach(panel, connector);
>> if (ret) {
>> dev_err(fsl_dev->dev, "failed to attach panel\n");