lkml.org 
[lkml]   [2023]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 2/9] drm/panel: Check for already prepared/enabled in drm_panel
    Date
    In a whole pile of panel drivers, we have code to make the
    prepare/unprepare/enable/disable callbacks behave as no-ops if they've
    already been called. It's silly to have this code duplicated
    everywhere. Add it to the core instead so that we can eventually
    delete it from all the drivers. Note: to get some idea of the
    duplicated code, try:
    git grep 'if.*>prepared' -- drivers/gpu/drm/panel
    git grep 'if.*>enabled' -- drivers/gpu/drm/panel

    NOTE: arguably, the right thing to do here is actually to skip this
    patch and simply remove all the extra checks from the individual
    drivers. Perhaps the checks were needed at some point in time in the
    past but maybe they no longer are? Certainly as we continue
    transitioning over to "panel_bridge" then we expect there to be much
    less variety in how these calls are made. When we're called as part of
    the bridge chain, things should be pretty simple. In fact, there was
    some discussion in the past about these checks [1], including a
    discussion about whether the checks were needed and whether the calls
    ought to be refcounted. At the time, I decided not to mess with it
    because it felt too risky.

    Looking closer at it now, I'm fairly certain that nothing in the
    existing codebase is expecting these calls to be refcounted. The only
    real question is whether someone is already doing something to ensure
    prepare()/unprepare() match and enabled()/disable() match. I would say
    that, even if there is something else ensuring that things match,
    there's enough complexity that adding an extra bool and an extra
    double-check here is a good idea. Let's add a drm_warn() to let people
    know that it's considered a minor error to take advantage of
    drm_panel's double-checking but we'll still make things work fine.

    [1] https://lore.kernel.org/r/20210416153909.v4.27.I502f2a92ddd36c3d28d014dd75e170c2d405a0a5@changeid

    Signed-off-by: Douglas Anderson <dianders@chromium.org>
    ---

    drivers/gpu/drm/drm_panel.c | 49 ++++++++++++++++++++++++++++++++-----
    include/drm/drm_panel.h | 14 +++++++++++
    2 files changed, 57 insertions(+), 6 deletions(-)

    diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
    index f634371c717a..4e1c4e42575b 100644
    --- a/drivers/gpu/drm/drm_panel.c
    +++ b/drivers/gpu/drm/drm_panel.c
    @@ -105,11 +105,22 @@ EXPORT_SYMBOL(drm_panel_remove);
    */
    int drm_panel_prepare(struct drm_panel *panel)
    {
    + int ret;
    +
    if (!panel)
    return -EINVAL;

    - if (panel->funcs && panel->funcs->prepare)
    - return panel->funcs->prepare(panel);
    + if (panel->prepared) {
    + dev_warn(panel->dev, "Skipping prepare of already prepared panel\n");
    + return 0;
    + }
    +
    + if (panel->funcs && panel->funcs->prepare) {
    + ret = panel->funcs->prepare(panel);
    + if (ret < 0)
    + return ret;
    + }
    + panel->prepared = true;

    return 0;
    }
    @@ -128,11 +139,22 @@ EXPORT_SYMBOL(drm_panel_prepare);
    */
    int drm_panel_unprepare(struct drm_panel *panel)
    {
    + int ret;
    +
    if (!panel)
    return -EINVAL;

    - if (panel->funcs && panel->funcs->unprepare)
    - return panel->funcs->unprepare(panel);
    + if (!panel->prepared) {
    + dev_warn(panel->dev, "Skipping unprepare of already unprepared panel\n");
    + return 0;
    + }
    +
    + if (panel->funcs && panel->funcs->unprepare) {
    + ret = panel->funcs->unprepare(panel);
    + if (ret < 0)
    + return ret;
    + }
    + panel->prepared = false;

    return 0;
    }
    @@ -155,11 +177,17 @@ int drm_panel_enable(struct drm_panel *panel)
    if (!panel)
    return -EINVAL;

    + if (panel->enabled) {
    + dev_warn(panel->dev, "Skipping enable of already enabled panel\n");
    + return 0;
    + }
    +
    if (panel->funcs && panel->funcs->enable) {
    ret = panel->funcs->enable(panel);
    if (ret < 0)
    return ret;
    }
    + panel->enabled = true;

    ret = backlight_enable(panel->backlight);
    if (ret < 0)
    @@ -187,13 +215,22 @@ int drm_panel_disable(struct drm_panel *panel)
    if (!panel)
    return -EINVAL;

    + if (!panel->enabled) {
    + dev_warn(panel->dev, "Skipping disable of already disabled panel\n");
    + return 0;
    + }
    +
    ret = backlight_disable(panel->backlight);
    if (ret < 0)
    DRM_DEV_INFO(panel->dev, "failed to disable backlight: %d\n",
    ret);

    - if (panel->funcs && panel->funcs->disable)
    - return panel->funcs->disable(panel);
    + if (panel->funcs && panel->funcs->disable) {
    + ret = panel->funcs->disable(panel);
    + if (ret < 0)
    + return ret;
    + }
    + panel->enabled = false;

    return 0;
    }
    diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
    index 432fab2347eb..c6cf75909389 100644
    --- a/include/drm/drm_panel.h
    +++ b/include/drm/drm_panel.h
    @@ -198,6 +198,20 @@ struct drm_panel {
    * the panel is powered up.
    */
    bool prepare_prev_first;
    +
    + /**
    + * @prepared:
    + *
    + * If true then the panel has been prepared.
    + */
    + bool prepared;
    +
    + /**
    + * @enabled:
    + *
    + * If true then the panel has been enabled.
    + */
    + bool enabled;
    };

    void drm_panel_init(struct drm_panel *panel, struct device *dev,
    --
    2.40.1.698.g37aff9b760-goog
    \
     
     \ /
      Last update: 2023-05-23 21:32    [W:4.044 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site