diff mbox series

drm/crtc-helper: Add store the property value

Message ID 20190117085044.19784-1-hoegeun.kwon@samsung.com (mailing list archive)
State New, archived
Headers show
Series drm/crtc-helper: Add store the property value | expand

Commit Message

Hoegeun Kwon Jan. 17, 2019, 8:50 a.m. UTC
There is a problem in crtc_helper that property value is not updated
when dpms is turned on or off. So modify the property value when dpms
is on.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 drivers/gpu/drm/drm_crtc_helper.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Daniel Vetter Jan. 17, 2019, 9:20 a.m. UTC | #1
On Thu, Jan 17, 2019 at 05:50:44PM +0900, Hoegeun Kwon wrote:
> There is a problem in crtc_helper that property value is not updated
> when dpms is turned on or off. So modify the property value when dpms
> is on.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>

This is fixed with atomic, and exynos is atomic. Why do you care about
this?
-Daniel

> ---
>  drivers/gpu/drm/drm_crtc_helper.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index a3c81850e755..57d359f0725c 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -735,6 +735,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>  				DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id,
>  					      set->connectors[i]->name);
>  				set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
> +
> +				drm_object_property_set_value(&set->connectors[i]->base,
> +							set->connectors[i]->dev->mode_config.dpms_property,
> +							DRM_MODE_DPMS_ON);
>  			}
>  		}
>  		__drm_helper_disable_unused_functions(dev);
> -- 
> 2.17.1
>
Hoegeun Kwon Jan. 17, 2019, 9:57 a.m. UTC | #2
On 1/17/19 6:20 PM, Daniel Vetter wrote:
> On Thu, Jan 17, 2019 at 05:50:44PM +0900, Hoegeun Kwon wrote:
>> There is a problem in crtc_helper that property value is not updated
>> when dpms is turned on or off. So modify the property value when dpms
>> is on.
>>
>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> This is fixed with atomic, and exynos is atomic. Why do you care about
> this?
> -Daniel


Thank you Daniel.

That's right, there is no problem with exynos because it uses atomic.

But I think it could be a problem with other connectors that do not use 
atoms.


Best regards,

Hoegeun


>
>> ---
>>   drivers/gpu/drm/drm_crtc_helper.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
>> index a3c81850e755..57d359f0725c 100644
>> --- a/drivers/gpu/drm/drm_crtc_helper.c
>> +++ b/drivers/gpu/drm/drm_crtc_helper.c
>> @@ -735,6 +735,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>>   				DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id,
>>   					      set->connectors[i]->name);
>>   				set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
>> +
>> +				drm_object_property_set_value(&set->connectors[i]->base,
>> +							set->connectors[i]->dev->mode_config.dpms_property,
>> +							DRM_MODE_DPMS_ON);
>>   			}
>>   		}
>>   		__drm_helper_disable_unused_functions(dev);
>> -- 
>> 2.17.1
>>
Daniel Vetter Jan. 17, 2019, 12:15 p.m. UTC | #3
On Thu, Jan 17, 2019 at 10:57 AM Hoegeun Kwon <hoegeun.kwon@samsung.com> wrote:
>
>
> On 1/17/19 6:20 PM, Daniel Vetter wrote:
> > On Thu, Jan 17, 2019 at 05:50:44PM +0900, Hoegeun Kwon wrote:
> >> There is a problem in crtc_helper that property value is not updated
> >> when dpms is turned on or off. So modify the property value when dpms
> >> is on.
> >>
> >> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> > This is fixed with atomic, and exynos is atomic. Why do you care about
> > this?
> > -Daniel
>
>
> Thank you Daniel.
>
> That's right, there is no problem with exynos because it uses atomic.
>
> But I think it could be a problem with other connectors that do not use
> atoms.

Yeah, but not sure we care about those drivers all that much. If
someone does, probably better to convert them to atomic (which is
still happening). We did have the equivalent of your patch in the i915
legacy modeset code, but it was quite tricky to get right. Much easier
with atomic, where properties have the right value by design.
-Daniel

>
> Best regards,
>
> Hoegeun
>
>
> >
> >> ---
> >>   drivers/gpu/drm/drm_crtc_helper.c | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> >> index a3c81850e755..57d359f0725c 100644
> >> --- a/drivers/gpu/drm/drm_crtc_helper.c
> >> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> >> @@ -735,6 +735,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
> >>                              DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id,
> >>                                            set->connectors[i]->name);
> >>                              set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
> >> +
> >> +                            drm_object_property_set_value(&set->connectors[i]->base,
> >> +                                                    set->connectors[i]->dev->mode_config.dpms_property,
> >> +                                                    DRM_MODE_DPMS_ON);
> >>                      }
> >>              }
> >>              __drm_helper_disable_unused_functions(dev);
> >> --
> >> 2.17.1
> >>
Hoegeun Kwon Jan. 18, 2019, 1:58 a.m. UTC | #4
On 1/17/19 9:15 PM, Daniel Vetter wrote:
> On Thu, Jan 17, 2019 at 10:57 AM Hoegeun Kwon <hoegeun.kwon@samsung.com> wrote:
>>
>> On 1/17/19 6:20 PM, Daniel Vetter wrote:
>>> On Thu, Jan 17, 2019 at 05:50:44PM +0900, Hoegeun Kwon wrote:
>>>> There is a problem in crtc_helper that property value is not updated
>>>> when dpms is turned on or off. So modify the property value when dpms
>>>> is on.
>>>>
>>>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
>>> This is fixed with atomic, and exynos is atomic. Why do you care about
>>> this?
>>> -Daniel
>>
>> Thank you Daniel.
>>
>> That's right, there is no problem with exynos because it uses atomic.
>>
>> But I think it could be a problem with other connectors that do not use
>> atoms.
> Yeah, but not sure we care about those drivers all that much. If
> someone does, probably better to convert them to atomic (which is
> still happening). We did have the equivalent of your patch in the i915
> legacy modeset code, but it was quite tricky to get right. Much easier
> with atomic, where properties have the right value by design.
> -Daniel


Thank you for the detailed explanation.

Please ignore this patch.


Best regards,

Hoegeun


>
>> Best regards,
>>
>> Hoegeun
>>
>>
>>>> ---
>>>>    drivers/gpu/drm/drm_crtc_helper.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
>>>> index a3c81850e755..57d359f0725c 100644
>>>> --- a/drivers/gpu/drm/drm_crtc_helper.c
>>>> +++ b/drivers/gpu/drm/drm_crtc_helper.c
>>>> @@ -735,6 +735,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>>>>                               DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id,
>>>>                                             set->connectors[i]->name);
>>>>                               set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
>>>> +
>>>> +                            drm_object_property_set_value(&set->connectors[i]->base,
>>>> +                                                    set->connectors[i]->dev->mode_config.dpms_property,
>>>> +                                                    DRM_MODE_DPMS_ON);
>>>>                       }
>>>>               }
>>>>               __drm_helper_disable_unused_functions(dev);
>>>> --
>>>> 2.17.1
>>>>
>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index a3c81850e755..57d359f0725c 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -735,6 +735,10 @@  int drm_crtc_helper_set_config(struct drm_mode_set *set,
 				DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id,
 					      set->connectors[i]->name);
 				set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
+
+				drm_object_property_set_value(&set->connectors[i]->base,
+							set->connectors[i]->dev->mode_config.dpms_property,
+							DRM_MODE_DPMS_ON);
 			}
 		}
 		__drm_helper_disable_unused_functions(dev);