All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
To: "puthik@chromium.org" <puthik@chromium.org>
Cc: "Vetter, Daniel" <daniel.vetter@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Tc, Jenny" <jenny.tc@intel.com>,
	"Weinehall, David" <david.weinehall@intel.com>
Subject: Re: [PATCH] drm/i915: Use AUX for backlight only if eDP 1.4 or later
Date: Tue, 1 Aug 2017 23:15:09 +0000	[thread overview]
Message-ID: <1501630535.31712.24.camel@dk-H97M-D3H> (raw)
In-Reply-To: <CANCcNXfcOMp90=mbqbD-=SiUhhq3Zv4-53SA+GmfO2j=37h10w@mail.gmail.com>




On Mon, 2017-07-31 at 15:41 -0700, Puthikorn Voravootivat wrote:
> > But now you're suggesting another arbitrary narrow selection of panels
> > based on limited evidence.
> 
> I understand your point that the panel I observe is not the
> representative of the real world.
> 
> My point is that we don't know that the panel will work or not unless
> we test all panel in the world.
> And blacklist would be too much work to maintain and whitelist would
> make this code too limited.
> As standard adoption should be better over time, I suggest that the
> newer panel should have
> better implement of the standard than older panel. And I suggest that
> eDP 1.4 should be a good
> heuristic for the "newer panel" based on these 2 reasons
> 
> 1. Even though it is a limited evident, David and I independently saw
> unrelated eDP 1.3 panel that
> implement this feature incorrectly.
> 2. eDP 1.4 is the first version that support AUX backlight enablement.
> TCON vendor probably also
> make sure the AUX backlight brightness ajustment works when testing
> that feature.
> 
> Is this make sense?
> 
> Thanks.
> 

I tried to investigate this a little bit and found a device that
reproduces the issue. The backlight does not come back up after a
suspend-resume cycle because the PWM controller does not get enabled at
resume. However, things just work at boot because the BIOS happens to
enable PCH PWM at boot and the panel lights up via the BL_PWM_PIN. Like
you said, this could be because some eDP 1.3 panels have a broken
implementation and eDP 1.4 panels are better. Or, with the BL_PWM_PIN
wired to the board, it simply overrides the DPCD settings. I decided to
not disconnect the PWM pin and test this theory since this was a
development laptop. In summary, I am not really sure blacklisting all
eDP 1.3 panels is the best idea. Also, I don't know how many eDP 1.4
panels this has been tested to correctly work on.

Anyway, since we have four panels that do not work, we could check if
these are the same model/make etc. and blacklist them if there's a
common thread.   

-DK

> On Mon, Jul 31, 2017 at 3:55 AM, Jani Nikula
> <jani.nikula@linux.intel.com> wrote:
> > On Mon, 24 Jul 2017, Puthikorn Voravootivat <puthik@chromium.org> wrote:
> >> I saw a DP 1.3 panel that advertise AUX backlight brightness control
> >> but not working properly. So it should work but not in real world.
> >> I think that is good reason enough to add this as a heuristic.
> >
> > Are you suggesting the one panel you mention is representative of the
> > real world?
> >
> > Granted, the original aux backlight implementation supported a very
> > narrow selection of panels. I believe this is the very reason you are
> > working on this patch series.
> >
> > But now you're suggesting another arbitrary narrow selection of panels
> > based on limited evidence.
> >
> >
> > BR,
> > Jani.
> >
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2017-08-01 23:15 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-19 10:29 [PATCH] drm/i915: Use AUX for backlight only if eDP 1.4 or later Jenny TC
2017-07-19 11:50 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-07-19 18:22 ` [PATCH] " Pandiyan, Dhinakaran
2017-07-20  9:33   ` Jani Nikula
2017-07-20 10:06     ` Tc, Jenny
2017-07-20 20:27       ` Pandiyan, Dhinakaran
2017-07-24 23:15         ` Puthikorn Voravootivat
2017-07-25 12:41           ` David Weinehall
2017-07-25 13:11             ` David Weinehall
2017-07-31 10:55           ` Jani Nikula
2017-07-31 22:41             ` Puthikorn Voravootivat
2017-08-01 23:15               ` Pandiyan, Dhinakaran [this message]
2017-08-02 13:53                 ` David Weinehall
2017-07-20 15:35     ` David Weinehall
2017-07-20 18:20       ` Pandiyan, Dhinakaran

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=1501630535.31712.24.camel@dk-H97M-D3H \
    --to=dhinakaran.pandiyan@intel.com \
    --cc=daniel.vetter@intel.com \
    --cc=david.weinehall@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jenny.tc@intel.com \
    --cc=puthik@chromium.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.