linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: Rajeev Nandan <rajeevny@codeaurora.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	freedreno <freedreno@lists.freedesktop.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Rob Clark <robdclark@gmail.com>, Lyude Paul <lyude@redhat.com>,
	Jani Nikula <jani.nikula@intel.com>,
	Rob Herring <robh@kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Daniel Thompson <daniel.thompson@linaro.org>,
	"Kristian H. Kristensen" <hoegsberg@chromium.org>,
	Abhinav Kumar <abhinavk@codeaurora.org>,
	Sean Paul <seanpaul@chromium.org>,
	Kalyan Thota <kalyan_t@codeaurora.org>,
	Krishna Manikandan <mkrishn@codeaurora.org>,
	Lee Jones <lee.jones@linaro.org>,
	Jingoo Han <jingoohan1@gmail.com>,
	linux-fbdev@vger.kernel.org
Subject: Re: [v7 1/5] drm/panel: add basic DP AUX backlight support
Date: Tue, 22 Jun 2021 11:33:13 -0700	[thread overview]
Message-ID: <CAD=FV=WJiA+RxaQA9xt7Tik_2pCEJo0+6b39Di8cfnSWGuKkJQ@mail.gmail.com> (raw)
In-Reply-To: <20210621183828.GA918146@ravnborg.org>

Hi,

On Mon, Jun 21, 2021 at 11:38 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> > > I cannot see why you need the extra check on ->enabled?
> > > Would it be sufficient to check backlight_is_blank() only?
> >
> > This extra check on bl->enabled flag is added to avoid enabling/disabling
> > backlight again if it is already enabled/disabled.
> > Using this flag way can know the transition between backlight blank and
> > un-blank, and decide when to enable/disable the backlight.
>
> My point is that this should really not be needed, as it would cover up
> for some other bug whaere we try to do something twice that is not
> needed. But I am less certain here so if you think it is needed, keep
> it as is.

I haven't tested this myself, but I believe that it is needed. I don't
think the backlight update_status() function is like an enable/disable
function. I believe it can be called more than one time even while the
backlight is disabled. For instance, you can see that
backlight_update_status() just blindly calls through to update the
status. That function can be called for a number of reasons. Perhaps
Rajeev can put some printouts to confirm but I think that if the
backlight is "blanked" for whatever reason and you write to sysfs and
change the backlight level you'll still get called again even though
the backlight is still "disabled".

-Doug

  parent reply	other threads:[~2021-06-22 18:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-19 10:40 [v7 0/5] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20 Rajeev Nandan
2021-06-19 10:40 ` [v7 1/5] drm/panel: add basic DP AUX backlight support Rajeev Nandan
     [not found]   ` <20210620093141.GA703072@ravnborg.org>
2021-06-21  8:38     ` rajeevny
     [not found]       ` <20210621183828.GA918146@ravnborg.org>
2021-06-22 18:33         ` Doug Anderson [this message]
2021-06-23  6:15           ` rajeevny
2021-06-19 10:40 ` [v7 2/5] drm/panel-simple: Support DP AUX backlight Rajeev Nandan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAD=FV=WJiA+RxaQA9xt7Tik_2pCEJo0+6b39Di8cfnSWGuKkJQ@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=a.hajda@samsung.com \
    --cc=abhinavk@codeaurora.org \
    --cc=daniel.thompson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=hoegsberg@chromium.org \
    --cc=jani.nikula@intel.com \
    --cc=jingoohan1@gmail.com \
    --cc=kalyan_t@codeaurora.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lyude@redhat.com \
    --cc=mkrishn@codeaurora.org \
    --cc=rajeevny@codeaurora.org \
    --cc=robdclark@gmail.com \
    --cc=robh@kernel.org \
    --cc=sam@ravnborg.org \
    --cc=seanpaul@chromium.org \
    --cc=thierry.reding@gmail.com \
    --subject='Re: [v7 1/5] drm/panel: add basic DP AUX backlight support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).