> >> 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; + } /* 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
