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. On Thu, Jul 20, 2017 at 1:27 PM, Pandiyan, Dhinakaran <dhinakaran.pandiyan@xxxxxxxxx> wrote: > > > > On Thu, 2017-07-20 at 10:06 +0000, Tc, Jenny wrote: >> > >> With older panels, AUX pin for backlight doesn't work. >> > >> > What evidence do you have to back up that claim? >> >> Debugging further it's found that the panel I am having doesn't support AUX Backlight. >> >> cat /sys/kernel/debug/dri/0/eDP-1/i915_dpcd >> ... >> 0701: bb ff 00 00 >> .... >> >> With below change its working for my panel. But doesn't address issue reported in >> https://bugs.freedesktop.org/show_bug.cgi?id=101619 which seems to have a wrong DPCD data. >> Since we don't have a proper fix for all panels, I agree for revert. >> >> intel_dp_aux_display_control_heuristic(struct intel_connector *connector) >> struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); >> uint8_t reg_val; >> >> + /* Panel dosn't support enabling AUX Backlight control */ >> + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) { >> + return false; >> + } > > AUX backlight brightness control should work even without AUX enable > capability. > > >> /* Panel doesn't support adjusting backlight brightness via PWN pin */ >> if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) >> return true; >> >> > >> On some panels, this causes backlight issues on S3 resume. >> > > >> > > What is it that we are missing in the resume path? >> > > >> > >> Enable the >> > >> feature only for eDP1.4 or later. >> > > >> > > I can't find the eDP 1.4 requirement in the spec. Regional brightness >> > > control was added in eDP 1.4, but we don't enable that. My concern is >> > > we might be missing a real fix by ignoring < eDP 1.4 panels. >> > >> > Agreed. >> > >> > This has been going on too long now, with no real effort to get at the roots of >> > the problem. The right approach is to revert the regressing commits now, >> > and start over. Reverts posted [1]. >> > >> > BR, >> > Jani. >> > >> > [1] https://patchwork.freedesktop.org/series/27623/ >> > >> > > >> > > >> > >> >> > >> Fix-suggested-by: Puthikorn Voravootivat <puthik@xxxxxxxxxxxx> >> > > >> > > 1. Please use the "Fixes" tag to refer to the commit that introduced >> > > the code you are fixing. >> > > 2. The "Suggested-by" tag is more common to give credits to the person >> > > who suggested the fix. >> > > 3. Please use the "Bugzilla" tag to point to the bug that David >> > > reported. >> > > >> > > >> > >> Signed-off-by: Jenny TC <jenny.tc@xxxxxxxxx> >> > >> --- >> > >> drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 6 +++++- >> > >> 1 file changed, 5 insertions(+), 1 deletion(-) >> > >> >> > >> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> > >> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> > >> index b25cd88..421f31f 100644 >> > >> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> > >> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> > >> @@ -292,7 +292,7 @@ static int intel_dp_aux_setup_backlight(struct >> > intel_connector *connector, >> > >> * via PWM pin or using AUX is better than using PWM pin. >> > >> * >> > >> * The heuristic to determine that using AUX pin is better than >> > >> using PWM pin is >> > >> - * that the panel support any of the feature list here. >> > >> + * that the panel is eDP 1.4 or later and support any of the feature >> > >> + list here >> > >> * - Regional backlight brightness adjustment >> > >> * - Backlight PWM frequency set >> > >> * - More than 8 bits resolution of brightness level @@ -310,6 >> > >> +310,10 @@ static int intel_dp_aux_setup_backlight(struct >> > intel_connector *connector, >> > >> if (!(intel_dp->edp_dpcd[2] & >> > DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) >> > >> return true; >> > >> >> > >> + /* Enable this for eDP 1.4 panel or later. */ >> > >> + if (intel_dp->edp_dpcd[0] < DP_EDP_14) >> > >> + return false; >> > >> + >> > >> /* Panel supports regional backlight brightness adjustment */ >> > >> if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_GENERAL_CAP_3, >> > >> ®_val) != 1) { >> > >> > -- >> > Jani Nikula, Intel Open Source Technology Center >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
