From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kmu-office.ch ([178.209.48.109]:45195 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752655AbdKIN7z (ORCPT ); Thu, 9 Nov 2017 08:59:55 -0500 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Thu, 09 Nov 2017 14:55:40 +0100 From: Stefan Agner To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org, alison.wang@freescale.com, stable@vger.kernel.org, Laurent Pinchart Subject: Re: [PATCH] drm/fsl-dcu: set DPMS off before initializing connector In-Reply-To: <6781308.LVpIJFO4VK@avalon> References: <20171109091436.622-1-stefan@agner.ch> <6781308.LVpIJFO4VK@avalon> Message-ID: <56ed16656438d7b7429e9639aa8e3367@agner.ch> Sender: stable-owner@vger.kernel.org List-ID: 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@vger.kernel.org >> Signed-off-by: Laurent Pinchart >> Signed-off-by: Stefan Agner >> --- >> 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");