Re: [nexus7-flo] backlight brightness

Re: [nexus7-flo] backlight brightness

[Rob Clark]

On Fri, Apr 29, 2016 at 1:32 AM, Vinay Simha <simhavcs@gmail.com> wrote:
> hi,
>
> In nx7, backlight brightness control is controlled by dcs commands of
> MIPI_DCS_SET_DISPLAY_BRIGHTNESS.
> there is no PWM going to panel interface directly as we have in ifc6410/nx5

afaiu it is not uncommon to need to control backlight via panel (ie, I
think basically all adaptive-backlight panels).  I think the answer is
for these panels, the panel driver would register it's own backlight
driver, rather than get a (pwm, etc) backlight via dt bindings.

Possibly if the backlight commands are standardized enough then we
could implement a single drm_panel_dsi_backlight helper which could be
created by any of the dsi adaptive-backlight panels (ie. pass in a
'struct mipi_dsi_device *').. something like:

struct drm_panel_dsi_backlight {
    struct mipi_panel_dsi_device *link;
    ...
}

struct backlight_device *drm_panel_create_dsi_backlight(struct
mipi_panel_dsi_device *link)
{
    struct drm_panel_dsi_backlight *dsi_bl;
    ...
    return devm_backlight_device_register(&link->dev, ..., dsi_bl,
dsi_bl_ops, props);
}

BR,
-R
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [nexus7-flo] backlight brightness

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1538 bytes --]

On Fri, Apr 29, 2016 at 08:04:09AM -0400, Rob Clark wrote:
> On Fri, Apr 29, 2016 at 1:32 AM, Vinay Simha <simhavcs@gmail.com> wrote:
> > hi,
> >
> > In nx7, backlight brightness control is controlled by dcs commands of
> > MIPI_DCS_SET_DISPLAY_BRIGHTNESS.
> > there is no PWM going to panel interface directly as we have in ifc6410/nx5
> 
> afaiu it is not uncommon to need to control backlight via panel (ie, I
> think basically all adaptive-backlight panels).  I think the answer is
> for these panels, the panel driver would register it's own backlight
> driver, rather than get a (pwm, etc) backlight via dt bindings.

Yes, that'd be my recommendation as well.

> Possibly if the backlight commands are standardized enough then we
> could implement a single drm_panel_dsi_backlight helper which could be
> created by any of the dsi adaptive-backlight panels (ie. pass in a
> 'struct mipi_dsi_device *').. something like:
> 
> struct drm_panel_dsi_backlight {
>     struct mipi_panel_dsi_device *link;
>     ...
> }
> 
> struct backlight_device *drm_panel_create_dsi_backlight(struct
> mipi_panel_dsi_device *link)
> {
>     struct drm_panel_dsi_backlight *dsi_bl;
>     ...
>     return devm_backlight_device_register(&link->dev, ..., dsi_bl,
> dsi_bl_ops, props);
> }

I agree it would make sense to have a DCS specific helper for backlight.
I'm somewhat sceptical about the standardization, even though these seem
to be part of DCS 1.3. But perhaps we can start by being optimistic.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [nexus7-flo] backlight brightness

[Vinay Simha]

On Mon, May 2, 2016 at 12:56 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Fri, Apr 29, 2016 at 08:04:09AM -0400, Rob Clark wrote:
>> On Fri, Apr 29, 2016 at 1:32 AM, Vinay Simha <simhavcs@gmail.com> wrote:
>> > hi,
>> >
>> > In nx7, backlight brightness control is controlled by dcs commands of
>> > MIPI_DCS_SET_DISPLAY_BRIGHTNESS.
>> > there is no PWM going to panel interface directly as we have in ifc6410/nx5
>>
>> afaiu it is not uncommon to need to control backlight via panel (ie, I
>> think basically all adaptive-backlight panels).  I think the answer is
>> for these panels, the panel driver would register it's own backlight
>> driver, rather than get a (pwm, etc) backlight via dt bindings.
>
> Yes, that'd be my recommendation as well.
>
>> Possibly if the backlight commands are standardized enough then we
>> could implement a single drm_panel_dsi_backlight helper which could be
>> created by any of the dsi adaptive-backlight panels (ie. pass in a
>> 'struct mipi_dsi_device *').. something like:
>>
>> struct drm_panel_dsi_backlight {
>>     struct mipi_panel_dsi_device *link;
>>     ...
>> }
>>
>> struct backlight_device *drm_panel_create_dsi_backlight(struct
>> mipi_panel_dsi_device *link)
>> {
>>     struct drm_panel_dsi_backlight *dsi_bl;
>>     ...
>>     return devm_backlight_device_register(&link->dev, ..., dsi_bl,
>> dsi_bl_ops, props);
>> }

struct backlight_device *devm_backlight_device_register(struct device *dev,
        const char *name, struct device *parent, void *devdata,
        const struct backlight_ops *ops,
        const struct backlight_properties *props)

struct device *dev = &jdi->dsi->dev;

 devm_backlight_device_register(
                                 dev,
                                 dev_name(dev),
                                 dev,
                                 jdi,
                                 &dsi_bl_ops, &props);

Here does the parent also should be jdi->dsi->dev ? because the
backlight is not mapped to fb of android ( android->
settings->brightness) , even though i can change the brightness value
from sysfs and it works only when the display is off. When it
awakes(power button press) panel_enable calls, backlight_update_status
will have the latest brightness value.

if echo the brightness value when the display is awake, dsi fails for
brightness control.
echo 100 > /sys/class/backlight/4700000.qcom,mdss_dsi.0/brightness
[  152.865949] dsi_cmds2buf_tx: cmd dma tx failed, type=0x15, data0=0x51, len=4

static int dsi_bl_update_status(struct backlight_device *bl)
{
        struct jdi_panel *jdi = bl_get_data(bl);
        struct mipi_dsi_device *dsi = jdi->dsi;
        int ret;
        u8 brightness = bl->props.brightness;

        ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
                                &brightness, sizeof(brightness));
        if (ret < 0)
                return ret;

        return 0;
}

static const struct backlight_ops dsi_bl_ops = {
        .update_status = dsi_bl_update_status,
 };

john,
for android fb do we need to map the backlight in userspace? As i have
seen in 3.4 kernel when we register the backlight using
platform_device_register, backlight used to map with the
userspace(ubuntu/fedora) settings->brightness.

>
> I agree it would make sense to have a DCS specific helper for backlight.
> I'm somewhat sceptical about the standardization, even though these seem
> to be part of DCS 1.3. But perhaps we can start by being optimistic.
>
> Thierry



-- 
Regards,

Vinay Simha.B.N.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [nexus7-flo] backlight brightness

[John Stultz]

On Mon, May 2, 2016 at 9:56 PM, Vinay Simha <simhavcs@gmail.com> wrote:
> Here does the parent also should be jdi->dsi->dev ? because the
> backlight is not mapped to fb of android ( android->
> settings->brightness) , even though i can change the brightness value
> from sysfs and it works only when the display is off. When it
> awakes(power button press) panel_enable calls, backlight_update_status
> will have the latest brightness value.
>
> if echo the brightness value when the display is awake, dsi fails for
> brightness control.
> echo 100 > /sys/class/backlight/4700000.qcom,mdss_dsi.0/brightness
> [  152.865949] dsi_cmds2buf_tx: cmd dma tx failed, type=0x15, data0=0x51, len=4
>
> static int dsi_bl_update_status(struct backlight_device *bl)
> {
>         struct jdi_panel *jdi = bl_get_data(bl);
>         struct mipi_dsi_device *dsi = jdi->dsi;
>         int ret;
>         u8 brightness = bl->props.brightness;
>
>         ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
>                                 &brightness, sizeof(brightness));
>         if (ret < 0)
>                 return ret;
>
>         return 0;
> }
>
> static const struct backlight_ops dsi_bl_ops = {
>         .update_status = dsi_bl_update_status,
>  };
>
> john,
> for android fb do we need to map the backlight in userspace? As i have
> seen in 3.4 kernel when we register the backlight using
> platform_device_register, backlight used to map with the
> userspace(ubuntu/fedora) settings->brightness.

Just to follow up here. I did add a lights HAL in my userspace build
to support the brightness sysfs interface. It looks like its working
ok, but as you note above, the brightness settings from the slider
don't seem to apply until the screen is turned off and back on.

thanks
-john
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel