On Tue, 01 Apr 2014, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> wrote:
> On Tue, 01 Apr 2014 10:19:29 +0300
> Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
>
>> On Mon, 31 Mar 2014, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> wrote:
>> > To make sure we properly follow the enable/disable sequences.
>> >
>> > Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/intel_dp.c | 62 ++++++++++++++++++++++++++++++++++++--
>> > drivers/gpu/drm/i915/intel_drv.h | 1 +
>> > drivers/gpu/drm/i915/intel_panel.c | 5 ++-
>> > 3 files changed, 65 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index bf73771..b6f7087 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -301,6 +301,20 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>> > return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
>> > }
>> >
>> > +static void assert_pwm(struct intel_connector *connector,
>> > + bool expected_state)
>> > +{
>> > + bool state;
>> > +
>> > + state = intel_panel_get_backlight(connector);
>>
>> If the duty cycle is regarded as a binary on/off, I'd rather add an
>> additional "is enabled" call to intel_panel.c. Especially so because the
>> duty cycle value returned by intel_panel_get_backlight is meaningless
>> without the max value.
>
> Hm I guess that would be cleaner; for my purposes I thought any
> non-zero PWM duty cycle would be sufficient, but of course other checks
> are needed as well, like whether the PWM enable bit is on, and checks
> against the BLC_EN bit in the PP regs, but those are logically
> separate. is_enabled might better map back to the PWM_EN bit rather
> than a non-zero duty cycle though.
We could add intel_panel_backlight_enabled() call that returns
connector->panel.backlight.enabled.
BR,
Jani.
>
>> >
>> > + if (I915_READ(VLV_BLC_PWM_CTL2(pipe) & BLM_PWM_ENABLE))
>> > + return 0;
>> > +
>>
>> If our internal state is consistent, I don't think this should be
>> necessary. And if our internal state isn't consistent, we should fix
>> that and maybe add internal asserts within intel_panel.c.
>
> Yeah this could be covered with other asserts as long as we have them
> in all the right places.
>
> --
> Jesse Barnes, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
