Andrzej Hajda <a.hajda@xxxxxxxxxxx> writes:
> On 21.09.2017 19:06, Sean Paul wrote:
>> This patch adds state tracking to the drm_panel functions which keep
>> track of enabled and prepared. If the calls are unbalanced, a WARNING is
>> issued.
>>
>> The motivation for this change is that a number of panel drivers
>> (including panel-simple) all do this to protect their regulator
>> refcounts. The atomic framework ensures the calls are balanced, and
>> there aren't any panel drivers being used by legacy drivers. As such,
>> these checks are useless, but let's add a WARNING just in case something
>> crazy happens (like a legacy driver using a panel).
>>
>> Less code == better.
>>
>> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
>
> I wonder if the tracking is needed at all, panels power states are
> usually the same as states of encoders they are connected to.
> But it is just remark to consider, no strong opposition :)
>
>> ---
>> drivers/gpu/drm/drm_panel.c | 2 ++
>> include/drm/drm_panel.h | 38 ++++++++++++++++++++++++++++++++++----
>> 2 files changed, 36 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
>> index 308d442a531b..9515219d3d2c 100644
>> --- a/drivers/gpu/drm/drm_panel.c
>> +++ b/drivers/gpu/drm/drm_panel.c
>> @@ -48,6 +48,8 @@ static LIST_HEAD(panel_list);
>> void drm_panel_init(struct drm_panel *panel)
>> {
>> INIT_LIST_HEAD(&panel->list);
>> + panel->enabled = false;
>> + panel->prepared = false;
>> }
>> EXPORT_SYMBOL(drm_panel_init);
>>
>> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
>> index 14ac240a1f64..b9a86a4cf29c 100644
>> --- a/include/drm/drm_panel.h
>> +++ b/include/drm/drm_panel.h
>> @@ -24,6 +24,7 @@
>> #ifndef __DRM_PANEL_H__
>> #define __DRM_PANEL_H__
>>
>> +#include <linux/bug.h>
>> #include <linux/errno.h>
>> #include <linux/list.h>
>>
>> @@ -84,6 +85,8 @@ struct drm_panel_funcs {
>> * @dev: parent device of the panel
>> * @funcs: operations that can be performed on the panel
>> * @list: panel entry in registry
>> + * @enabled: keeps track of the panel enabled status
>> + * @prepared: keeps track of the panel prepared status
>> */
>> struct drm_panel {
>> struct drm_device *drm;
>> @@ -93,6 +96,9 @@ struct drm_panel {
>> const struct drm_panel_funcs *funcs;
>>
>> struct list_head list;
>> +
>> + bool enabled;
>> + bool prepared;
>> };
>>
>> /**
>> @@ -104,12 +110,18 @@ struct drm_panel {
>> * is usually no longer possible to communicate with the panel until another
>> * call to drm_panel_prepare().
>> *
>> + * Atomic framework should ensure that prepare/unprepare are properly balanced.
>> + * If this is not the case, a WARNING will be issued.
>> + *
>> * Return: 0 on success or a negative error code on failure.
>> */
>> static inline int drm_panel_unprepare(struct drm_panel *panel)
>> {
>> - if (panel && panel->funcs && panel->funcs->unprepare)
>> + if (panel && panel->funcs && panel->funcs->unprepare) {
>> + WARN_ON(!panel->prepared);
>
> WARN_ON(!panel->prepared || panel->enabled);
>
> Similar double checks should be used in other places.
I like this idea! I feel like I've seen enough drivers that didn't
balance prepare/unprepare and enable/disable because for their one panel
they supported it didn't matter.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
