[PATCH V6 3/8] drm/panel: simple: Add support for auo_b133htn01 panel

[Ajay kumar]

On Wed, Jul 30, 2014 at 7:00 PM, Thierry Reding
<thierry.reding at gmail.com> wrote:
> On Wed, Jul 30, 2014 at 05:02:11PM +0530, Ajay kumar wrote:
>> On Wed, Jul 30, 2014 at 4:21 PM, Thierry Reding
>> <thierry.reding at gmail.com> wrote:
>> > On Sat, Jul 26, 2014 at 12:52:05AM +0530, Ajay Kumar wrote:
>> >> Add panel_desc structure for auo_b133htn01 eDP panel.
>> >>
>> >> Also, modify the panel_simple routines to support timing_parameter
>> >> delays if mentioned in the panel_desc structure.
>> >>
>> >> Signed-off-by: Ajay Kumar <ajaykumar.rs at samsung.com>
>> >> ---
>> >>  .../devicetree/bindings/panel/auo,b133htn01.txt    |    7 +++
>> >>  drivers/gpu/drm/panel/panel-simple.c               |   47 ++++++++++++++++++++
>> >>  2 files changed, 54 insertions(+)
>> >>  create mode 100644 Documentation/devicetree/bindings/panel/auo,b133htn01.txt
>> >
>> > I think this should be two patches, one which adds the delay parameters
>> > and another which adds support for the new panel.
>> Ok. Will split it.
>
> I was going to propose that I split this up myself so you no longer have
> to worry about it. I can easily apply the changes we discussed. But if
> you prefer to keep it in one series that works for me too.
Well, you can only take in the "delay" structure as of now.

Ajay
>> > In general I think it would be good to have a description of these in
>> > the struct panel_desc structure, something like this perhaps:
>> >
>> >         /**
>> >          * @prepare: the time (in milliseconds) that it takes for the panel
>> >          *           to become ready and start receiving video data
>> >          * @enable: the time (in milliseconds) that it takes for the panel
>> >          *          to display the first valid frame after starting to
>> >          *          receive video data
>> >          * @disable: the time (in milliseconds) that it takes for the panel
>> >          *           to turn the display off (no content is visible)
>> >          * @unprepare: the time (in milliseconds) that it takes for the panel
>>                         to power down itself completely.
>> >          */
>> >         struct {
>> >                 unsigned int prepare;
>> >                 unsigned int enable;
>> >                 unsigned int disable;
>> >                 unsigned int unprepare;
>> >         } delay;
>> >
>> > For prepare and enable delays this would mean that they should take
>> > effect at the very end of the .prepare() and .enable() functions,
>> > respectively. For disable in means that it should be at the end (for
>> > example to take into account the time it takes for backlight to
>> > completely turn off). For unprepare I have no idea what we would need it
>> > for. And the new panel that you're adding in this patch doesn't use it
>> > either, so perhaps it can just be left out (for now)?
>> Actually, there was a typo.
>> That should have been .unprepare_stage_delay = 50.
>> This is needed because panels need some delay before powering
>> them on again.
>> As in, assume you are doing a test to turn on/off display continuously,
>> Then, the delay between
>> (N - 1)th cycle poweroff to Nth cycle poweron should be at least 500ms.
>> That's what the datasheet says! And, somehow 50ms works fine for me.
>
> Okay, that makes sense then.
>
> Thierry