Re: [Intel-gfx] [PATCH 19/30] drm/dp: Pass drm_dp_aux to drm_dp_link_train_clock_recovery_delay()
On Fri, Feb 19, 2021 at 04:53:15PM -0500, Lyude Paul wrote:
> So that we can start using drm_dbg_*() in
> drm_dp_link_train_clock_recovery_delay().
>
> Signed-off-by: Lyude Paul
I wonder if we could have a drm_dp so we encapsulate both aux and dpcd
related information...
But this one already solves the issue...
Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 2 +-
> drivers/gpu/drm/drm_dp_helper.c | 3 ++-
> drivers/gpu/drm/i915/display/intel_dp_link_training.c | 2 +-
> drivers/gpu/drm/msm/dp/dp_ctrl.c | 2 +-
> drivers/gpu/drm/msm/edp/edp_ctrl.c| 2 +-
> drivers/gpu/drm/radeon/atombios_dp.c | 2 +-
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
> include/drm/drm_dp_helper.h | 4 +++-
> 8 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> index 6d35da65e09f..4468f9d6b4dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> @@ -611,7 +611,7 @@ amdgpu_atombios_dp_link_train_cr(struct
> amdgpu_atombios_dp_link_train_info *dp_i
> dp_info->tries = 0;
> voltage = 0xff;
> while (1) {
> - drm_dp_link_train_clock_recovery_delay(dp_info->dpcd);
> + drm_dp_link_train_clock_recovery_delay(dp_info->aux,
> dp_info->dpcd);
>
> if (drm_dp_dpcd_read_link_status(dp_info->aux,
>dp_info->link_status) <= 0) {
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 194e0c273809..ce08eb3bface 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -132,7 +132,8 @@ u8 drm_dp_get_adjust_request_post_cursor(const u8
> link_status[DP_LINK_STATUS_SIZ
> }
> EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor);
>
> -void drm_dp_link_train_clock_recovery_delay(const u8
> dpcd[DP_RECEIVER_CAP_SIZE])
> +void drm_dp_link_train_clock_recovery_delay(const struct drm_dp_aux *aux,
> + const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> {
> unsigned long rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
>DP_TRAINING_AUX_RD_MASK;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index 892d7db7d94f..222073d46bdb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -441,7 +441,7 @@ static void
> intel_dp_link_training_clock_recovery_delay(struct intel_dp *intel_d
> enum drm_dp_phy dp_phy)
> {
> if (dp_phy == DP_PHY_DPRX)
> - drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd);
> + drm_dp_link_train_clock_recovery_delay(&intel_dp->aux,
> intel_dp->dpcd);
> else
> drm_dp_lttpr_link_train_clock_recovery_delay();
> }
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 36b39c381b3f..2501a6b326a3 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1103,7 +1103,7 @@ static int dp_ctrl_link_train_1(struct dp_ctrl_private
> *ctrl,
> tries = 0;
> old_v_level = ctrl->link->phy_params.v_level;
> for (tries = 0; tries < maximum_retries; tries++) {
> - drm_dp_link_train_clock_recovery_delay(ctrl->panel->dpcd);
> + drm_dp_link_train_clock_recovery_delay(ctrl->aux,
> ctrl->panel->dpcd);
>
> ret = dp_ctrl_read_link_status(ctrl, link_status);
> if (ret)
> diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c
> b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> index 57af3d8b6699..6501598448b4 100644
> --- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
> +++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> @@ -608,7 +608,7 @@ static int edp_start_link_train_1(struct edp_ctrl *ctrl)
> tries = 0;
> old_v_level = ctrl->v_level;
> while (1) {
> - drm_dp_link_train_clock_recovery_delay(ctrl->dpcd);
> + drm_dp_link_train_clock_recovery_delay(ctrl->drm_aux,
> ctrl->dpcd);
>
> rlen = drm_dp_dpcd_read_link_status(ctrl->drm_aux, link_status);
> if (rlen < DP_LINK_STATUS_SIZE) {
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
> b/drivers/gpu/drm/radeon/atombios_dp.c
&
Re: [PATCH v3 2/2] drm/i915/icp+: Use icp_hpd_irq_setup() instead of spt_hpd_irq_setup()
On Tue, Feb 16, 2021 at 09:53:37PM -0500, Lyude Paul wrote: > While reviewing patches for handling workarounds related to gen9 bc, Imre > from Intel discovered that we're using spt_hpd_irq_setup() on ICP+ PCHs > despite it being almost the same as icp_hpd_irq_setup(). Since we need to > be calling icp_hpd_irq_setup() to ensure that CML-S/TGP platforms function > correctly anyway, let's move platforms using PCH_ICP which aren't handled > by gen11_hpd_irq_setup() over to icp_hpd_irq_setup(). > > Cc: Tejas Upadhyay > Signed-off-by: Lyude Paul makes sense to me... Reviewed-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/i915_irq.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index f86b147f588f..7ec61187a315 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -4320,6 +4320,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv) > dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; > else if (IS_GEN9_LP(dev_priv)) > dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; > + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) > + dev_priv->display.hpd_irq_setup = icp_hpd_irq_setup; > else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) > dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; > else > -- > 2.29.2 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 4/4] drm/i915/gen9_bc: Add W/A for missing STRAP config on TGP PCH + CML combos
On Wed, Feb 10, 2021 at 10:23:58PM -0500, Rodrigo Vivi wrote:
> On Tue, Feb 09, 2021 at 04:28:31PM -0500, Lyude Paul wrote:
> > Apparently the new gen9_bc platforms that Intel has introduced don't
> > provide us with a STRAP config register to read from for initializing DDI
> > B, C, and D detection. So, workaround this by hard-coding our strap config
> > in intel_setup_outputs().
> >
> > Changes since v4:
> > * Split this into it's own commit
> >
> > Cc: Matt Roper
> > Cc: Jani Nikula
> > Cc: Ville Syrjala
> > [originally from Tejas's work]
> > Signed-off-by: Tejas Upadhyay
> > Signed-off-by: Lyude Paul
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 9 -
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index beed08c00b6c..4dee37f8659d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -11943,7 +11943,14 @@ static void intel_setup_outputs(struct
> > drm_i915_private *dev_priv)
> >
> > /* DDI B, C, D, and F detection is indicated by the SFUSE_STRAP
> > * register */
> > - found = intel_de_read(dev_priv, SFUSE_STRAP);
> > + if (HAS_PCH_TGP(dev_priv)) {
> > + /* W/A due to lack of STRAP config on TGP PCH*/
> > + found = (SFUSE_STRAP_DDIB_DETECTED |
> > +SFUSE_STRAP_DDIC_DETECTED |
> > +SFUSE_STRAP_DDID_DETECTED);
>
> we have somewhere in this function these forced fuse straps for gen9
> platform...
> don't we have a ways to combine them?
>
> Afterall, the reason that we need these forced bits is
> because it is a gen9, not because it is a TGP...
just ignore my non-sense comment... I confused with the
/* WaIgnoreDDIAStrap: skl */
above...
thought it was for all the ports... not just for port A...
Reviewed-by: Rodrigo Vivi
>
> > + } else {
> > + found = intel_de_read(dev_priv, SFUSE_STRAP);
> > + }
> >
> > if (found & SFUSE_STRAP_DDIB_DETECTED)
> > intel_ddi_init(dev_priv, PORT_B);
> > --
> > 2.29.2
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v4 10/11] drm/dp: Extract i915's eDP backlight code into DRM helpers
On Mon, Feb 08, 2021 at 06:39:00PM -0500, Lyude Paul wrote:
> Since we're about to implement eDP backlight support in nouveau using the
> standard protocol from VESA, we might as well just take the code that's
> already written for this and move it into a set of shared DRM helpers.
>
> Note that these helpers are intended to handle DPCD related backlight
> control bits such as setting the brightness level over AUX, probing the
> backlight's TCON, enabling/disabling the backlight over AUX if supported,
> etc. Any PWM-related portions of backlight control are explicitly left up
> to the driver, as these will vary from platform to platform.
>
> The only exception to this is the calculation of the PWM frequency
> pre-divider value. This is because the only platform-specific information
> required for this is the PWM frequency of the panel, which the driver is
> expected to provide if available. The actual algorithm for calculating this
> value is standard and is defined in the eDP specification from VESA.
>
> Note that these helpers do not yet implement the full range of features
> the VESA backlight interface provides, and only provide the following
> functionality (all of which was already present in i915's DPCD backlight
> support):
>
> * Basic control of brightness levels
> * Basic probing of backlight capabilities
> * Helpers for enabling and disabling the backlight
>
> v3:
> * Split out changes to i915's backlight code to separate patches to make it
> easier to review
> v4:
> * Style/spelling changes from Thomas Zimmermann
>
> Signed-off-by: Lyude Paul
> Cc: Jani Nikula
> Cc: Dave Airlie
> Cc: greg.depo...@gmail.com
> ---
> drivers/gpu/drm/drm_dp_helper.c | 332 ++
> .../drm/i915/display/intel_display_types.h| 5 +-
> .../drm/i915/display/intel_dp_aux_backlight.c | 285 ++-
> include/drm/drm_dp_helper.h | 48 +++
> 4 files changed, 412 insertions(+), 258 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index eedbb48815b7..1a3d4679d720 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -3082,3 +3082,335 @@ int drm_dp_pcon_convert_rgb_to_ycbcr(struct
> drm_dp_aux *aux, u8 color_spc)
> return 0;
> }
> EXPORT_SYMBOL(drm_dp_pcon_convert_rgb_to_ycbcr);
> +
> +/**
> + * drm_edp_backlight_set_level() - Set the backlight level of an eDP panel
> via AUX
> + * @aux: The DP AUX channel to use
> + * @bl: Backlight capability info from drm_edp_backlight_init()
> + * @level: The brightness level to set
> + *
> + * Sets the brightness level of an eDP panel's backlight. Note that the
> panel's backlight must
> + * already have been enabled by the driver by calling
> drm_edp_backlight_enable().
> + *
> + * Returns: %0 on success, negative error code on failure
> + */
> +int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
> + u16 level)
> +{
> + int ret;
> + u8 buf[2] = { 0 };
> +
> + if (bl->lsb_reg_used) {
> + buf[0] = (level & 0xff00) >> 8;
> + buf[1] = (level & 0x00ff);
> + } else {
> + buf[0] = level;
> + }
> +
> + ret = drm_dp_dpcd_write(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, buf,
> sizeof(buf));
> + if (ret != sizeof(buf)) {
> + DRM_ERROR("%s: Failed to write aux backlight level: %d\n",
> aux->name, ret);
> + return ret < 0 ? ret : -EIO;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_edp_backlight_set_level);
> +
> +static int
> +drm_edp_backlight_set_enable(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
> + bool enable)
> +{
> + int ret;
> + u8 buf;
> +
> + /* The panel uses something other then DPCD for enabling its backlight
> */
> + if (!bl->aux_enable)
> + return 0;
> +
> + ret = drm_dp_dpcd_readb(aux, DP_EDP_DISPLAY_CONTROL_REGISTER, &buf);
> + if (ret != 1) {
> + DRM_ERROR("%s: Failed to read eDP display control register:
> %d\n", aux->name, ret);
> + return ret < 0 ? ret : -EIO;
> + }
> + if (enable)
> + buf |= DP_EDP_BACKLIGHT_ENABLE;
> + else
> + buf &= ~DP_EDP_BACKLIGHT_ENABLE;
> +
> + ret = drm_dp_dpcd_writeb(aux, DP_EDP_DISPLAY_CONTROL_REGISTER, buf);
> + if (ret != 1) {
> + DRM_ERROR("%s: Failed to write eDP display control register:
> %d\n", aux->name, ret);
> + return ret < 0 ? ret : -EIO;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * drm_edp_backlight_enable() - Enable an eDP panel's backlight using DPCD
> + * @aux: The DP AUX channel to use
> + * @bl: Backlight capability info from drm_edp_backlight_init()
> + * @level: The initial backlight level to set via AUX, if there is one
> + *
> + * This function handles enabling DPCD backlight controls on a panel over
> DP
Re: [RFC v4 05/11] drm/i915/dpcd_bl: Cleanup intel_dp_aux_vesa_enable_backlight() a bit
On Mon, Feb 08, 2021 at 06:38:55PM -0500, Lyude Paul wrote:
> Get rid of the extraneous switch case in here, and just open code
> edp_backlight_mode as we only ever use it once.
>
> v4:
> * Check that backlight mode is DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD, not
> DP_EDP_BACKLIGHT_CONTROL_MODE_MASK - imirkin
>
> Signed-off-by: Lyude Paul
Reviewed-by: Rodrigo Vivi
> ---
> .../gpu/drm/i915/display/intel_dp_aux_backlight.c | 15 ++-
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index c37ccc8538cb..57218faed4a3 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -382,7 +382,7 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> struct intel_panel *panel = &connector->panel;
> - u8 dpcd_buf, new_dpcd_buf, edp_backlight_mode;
> + u8 dpcd_buf, new_dpcd_buf;
> u8 pwmgen_bit_count = panel->backlight.edp.vesa.pwmgen_bit_count;
>
> if (drm_dp_dpcd_readb(&intel_dp->aux,
> @@ -393,12 +393,8 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
> }
>
> new_dpcd_buf = dpcd_buf;
> - edp_backlight_mode = dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
>
> - switch (edp_backlight_mode) {
> - case DP_EDP_BACKLIGHT_CONTROL_MODE_PWM:
> - case DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET:
> - case DP_EDP_BACKLIGHT_CONTROL_MODE_PRODUCT:
> + if ((dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) !=
> DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
> new_dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
> new_dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;
>
> @@ -406,13 +402,6 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
> pwmgen_bit_count) != 1)
> drm_dbg_kms(&i915->drm,
> "Failed to write aux pwmgen bit count\n");
> -
> - break;
> -
> - /* Do nothing when it is already DPCD mode */
> - case DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD:
> - default:
> - break;
> }
>
> if (panel->backlight.edp.vesa.pwm_freq_pre_divider) {
> --
> 2.29.2
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RFC v4 08/11] drm/i915/dpcd_bl: Return early in vesa_calc_max_backlight if we can't read PWMGEN_BIT_COUNT
On Mon, Feb 08, 2021 at 06:38:58PM -0500, Lyude Paul wrote:
> If we can't read DP_EDP_PWMGEN_BIT_COUNT in
> intel_dp_aux_vesa_calc_max_backlight() but do have a valid PWM frequency
> defined in the VBT, we'll keep going in the function until we inevitably
> fail on reading DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN. There's not much point in
> doing this, so just return early.
>
> Signed-off-by: Lyude Paul
Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index 611eb3a7cc08..a139f0e08839 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -449,11 +449,14 @@ static u32 intel_dp_aux_vesa_calc_max_backlight(struct
> intel_connector *connecto
> int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1;
> u8 pn, pn_min, pn_max;
>
> - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_PWMGEN_BIT_COUNT, &pn) ==
> 1) {
> - pn &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
> - max_backlight = (1 << pn) - 1;
> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_PWMGEN_BIT_COUNT, &pn) !=
> 1) {
> + drm_dbg_kms(&i915->drm, "Failed to read pwmgen bit count
> cap\n");
> + return 0;
> }
>
> + pn &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
> + max_backlight = (1 << pn) - 1;
> +
> /* Find desired value of (F x P)
>* Note that, if F x P is out of supported range, the maximum value or
>* minimum value will applied automatically. So no need to check that.
> --
> 2.29.2
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v4 09/11] drm/i915/dpcd_bl: Print return codes for VESA backlight failures
On Mon, Feb 08, 2021 at 06:38:59PM -0500, Lyude Paul wrote:
> Also, stop printing the DPCD register that failed, and just describe it
> instead. Saves us from having to look up each register offset when reading
> through kernel logs (plus, DPCD dumping with drm.debug |= 0x100 will give
> us that anyway).
>
> Signed-off-by: Lyude Paul
Reviewed-by: Rodrigo Vivi
> ---
> .../drm/i915/display/intel_dp_aux_backlight.c | 101 +-
> 1 file changed, 52 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index a139f0e08839..a98d9bd4b0ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -274,14 +274,12 @@ static bool
> intel_dp_aux_vesa_backlight_dpcd_mode(struct intel_connector *connec
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + int ret;
> u8 mode_reg;
>
> - if (drm_dp_dpcd_readb(&intel_dp->aux,
> - DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
> - &mode_reg) != 1) {
> - drm_dbg_kms(&i915->drm,
> - "Failed to read the DPCD register 0x%x\n",
> - DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
> + ret = drm_dp_dpcd_readb(&intel_dp->aux,
> DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &mode_reg);
> + if (ret != 1) {
> + drm_dbg_kms(&i915->drm, "Failed to read backlight mode: %d\n",
> ret);
> return false;
> }
>
> @@ -297,6 +295,7 @@ static u32 intel_dp_aux_vesa_get_backlight(struct
> intel_connector *connector, en
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + int ret;
> u8 read_val[2] = { 0x0 };
> u16 level = 0;
>
> @@ -307,10 +306,10 @@ static u32 intel_dp_aux_vesa_get_backlight(struct
> intel_connector *connector, en
> if (!intel_dp_aux_vesa_backlight_dpcd_mode(connector))
> return connector->panel.backlight.max;
>
> - if (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
> &read_val,
> - sizeof(read_val)) != sizeof(read_val)) {
> - drm_dbg_kms(&i915->drm, "Failed to read DPCD register 0x%x\n",
> - DP_EDP_BACKLIGHT_BRIGHTNESS_MSB);
> + ret = drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
> &read_val,
> +sizeof(read_val));
> + if (ret != sizeof(read_val)) {
> + drm_dbg_kms(&i915->drm, "Failed to read brightness level:
> %d\n", ret);
> return 0;
> }
>
> @@ -333,6 +332,7 @@ intel_dp_aux_vesa_set_backlight(const struct
> drm_connector_state *conn_state,
> struct intel_connector *connector =
> to_intel_connector(conn_state->connector);
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + int ret;
> u8 vals[2] = { 0x0 };
>
> /* Write the MSB and/or LSB */
> @@ -343,10 +343,10 @@ intel_dp_aux_vesa_set_backlight(const struct
> drm_connector_state *conn_state,
> vals[0] = level;
> }
>
> - if (drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
> vals,
> - sizeof(vals)) != sizeof(vals)) {
> - drm_dbg_kms(&i915->drm,
> - "Failed to write aux backlight level\n");
> + ret = drm_dp_dpcd_write(&intel_dp->aux,
> DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, vals,
> + sizeof(vals));
> + if (ret != sizeof(vals)) {
> + drm_dbg_kms(&i915->drm, "Failed to write aux backlight level:
> %d\n", ret);
> return;
> }
> }
> @@ -355,26 +355,28 @@ static void set_vesa_backlight_enable(struct
> intel_connector *connector, bool en
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + int ret;
> u8 reg_val = 0;
>
> /* Early return when display use other mechanism to enable backlight. */
> if (!connector->panel.backlight.edp.vesa.aux_enable)
> return;
>
> - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
&
Re: [Intel-gfx] [PATCH v5 2/4] drm/i915/gen9_bc: Introduce TGP PCH DDC pin mappings
On Tue, Feb 09, 2021 at 04:28:29PM -0500, Lyude Paul wrote:
> With the introduction of gen9_bc, where Intel combines Cometlake CPUs with
> a Tigerpoint PCH, we'll need to introduce new DDC pin mappings for this
> platform in order to make all of the display connectors work. So, let's do
> that.
>
> Changes since v4:
> * Split this into it's own patch - vsyrjala
>
> Cc: Matt Roper
> Cc: Jani Nikula
> Cc: Ville Syrjala
> [originally from Tejas's work]
> Signed-off-by: Tejas Upadhyay
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/i915/display/intel_bios.c | 9 +
> drivers/gpu/drm/i915/display/intel_hdmi.c | 20
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index 7118530a1c38..1289f9d437e4 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1638,6 +1638,12 @@ static const u8 adls_ddc_pin_map[] = {
> [ADLS_DDC_BUS_PORT_TC4] = GMBUS_PIN_12_TC4_ICP,
> };
>
> +static const u8 gen9bc_tgp_ddc_pin_map[] = {
> + [DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
> + [DDC_BUS_DDI_C] = GMBUS_PIN_9_TC1_ICP,
> + [DDC_BUS_DDI_D] = GMBUS_PIN_10_TC2_ICP,
> +};
> +
> static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin)
> {
> const u8 *ddc_pin_map;
> @@ -1651,6 +1657,9 @@ static u8 map_ddc_pin(struct drm_i915_private
> *dev_priv, u8 vbt_pin)
> } else if (IS_ROCKETLAKE(dev_priv) && INTEL_PCH_TYPE(dev_priv) ==
> PCH_TGP) {
> ddc_pin_map = rkl_pch_tgp_ddc_pin_map;
> n_entries = ARRAY_SIZE(rkl_pch_tgp_ddc_pin_map);
> + } else if (HAS_PCH_TGP(dev_priv) && IS_GEN9_BC(dev_priv)) {
> + ddc_pin_map = gen9bc_tgp_ddc_pin_map;
> + n_entries = ARRAY_SIZE(gen9bc_tgp_ddc_pin_map);
> } else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) {
> ddc_pin_map = icp_ddc_pin_map;
> n_entries = ARRAY_SIZE(icp_ddc_pin_map);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index dad54e116bc4..49528d07c7f3 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3138,6 +3138,24 @@ static u8 rkl_port_to_ddc_pin(struct drm_i915_private
> *dev_priv, enum port port)
> return GMBUS_PIN_1_BXT + phy;
> }
>
> +static u8 gen9bc_port_to_ddc_pin(struct drm_i915_private *i915, enum port
> port)
> +{
> + enum phy phy = intel_port_to_phy(i915, port);
> +
> + drm_WARN_ON(&i915->drm, port == PORT_A);
> +
> + /*
> + * Pin mapping for GEN9 BC depends on which PCH is present. With TGP,
> + * final two outputs use type-c pins, even though they're actually
> + * combo outputs. With CMP, the traditional DDI A-D pins are used for
> + * all outputs.
> + */
> + if (INTEL_PCH_TYPE(i915) >= PCH_TGP && phy >= PHY_C)
> + return GMBUS_PIN_9_TC1_ICP + phy - PHY_C;
> +
> + return GMBUS_PIN_1_BXT + phy;
> +}
> +
> static u8 dg1_port_to_ddc_pin(struct drm_i915_private *dev_priv, enum port
> port)
> {
> return intel_port_to_phy(dev_priv, port) + 1;
> @@ -3202,6 +3220,8 @@ static u8 intel_hdmi_ddc_pin(struct intel_encoder
> *encoder)
> ddc_pin = dg1_port_to_ddc_pin(dev_priv, port);
> else if (IS_ROCKETLAKE(dev_priv))
> ddc_pin = rkl_port_to_ddc_pin(dev_priv, port);
> + else if (IS_GEN9_BC(dev_priv) && HAS_PCH_TGP(dev_priv))
> + ddc_pin = gen9bc_port_to_ddc_pin(dev_priv, port);
what about also calling this function gen9bc_tgp_ ?
but up to you...
this version is much better without the if gen9 inside a "tgp" func...
thanks
Reviewed-by: Rodrigo Vivi
> else if (HAS_PCH_MCC(dev_priv))
> ddc_pin = mcc_port_to_ddc_pin(dev_priv, port);
> else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> --
> 2.29.2
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v5 4/4] drm/i915/gen9_bc: Add W/A for missing STRAP config on TGP PCH + CML combos
On Tue, Feb 09, 2021 at 04:28:31PM -0500, Lyude Paul wrote:
> Apparently the new gen9_bc platforms that Intel has introduced don't
> provide us with a STRAP config register to read from for initializing DDI
> B, C, and D detection. So, workaround this by hard-coding our strap config
> in intel_setup_outputs().
>
> Changes since v4:
> * Split this into it's own commit
>
> Cc: Matt Roper
> Cc: Jani Nikula
> Cc: Ville Syrjala
> [originally from Tejas's work]
> Signed-off-by: Tejas Upadhyay
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index beed08c00b6c..4dee37f8659d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11943,7 +11943,14 @@ static void intel_setup_outputs(struct
> drm_i915_private *dev_priv)
>
> /* DDI B, C, D, and F detection is indicated by the SFUSE_STRAP
>* register */
> - found = intel_de_read(dev_priv, SFUSE_STRAP);
> + if (HAS_PCH_TGP(dev_priv)) {
> + /* W/A due to lack of STRAP config on TGP PCH*/
> + found = (SFUSE_STRAP_DDIB_DETECTED |
> + SFUSE_STRAP_DDIC_DETECTED |
> + SFUSE_STRAP_DDID_DETECTED);
we have somewhere in this function these forced fuse straps for gen9 platform...
don't we have a ways to combine them?
Afterall, the reason that we need these forced bits is
because it is a gen9, not because it is a TGP...
> + } else {
> + found = intel_de_read(dev_priv, SFUSE_STRAP);
> + }
>
> if (found & SFUSE_STRAP_DDIB_DETECTED)
> intel_ddi_init(dev_priv, PORT_B);
> --
> 2.29.2
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 3/4] drm/i915/gen9_bc: Introduce HPD pin mappings for TGP PCH + CML combos
On Tue, Feb 09, 2021 at 04:28:30PM -0500, Lyude Paul wrote:
> Next, let's start introducing the HPD pin mappings for Intel's new gen9_bc
> platform in order to make hotplugging display connectors work. Since
> gen9_bc is just a TGP PCH along with a CML CPU, except with the same HPD
> mappings as ICL, we simply add a skl_hpd_pin function that is shared
> between gen9 and gen9_bc which handles both the traditional gen9 HPD pin
> mappings and the Icelake HPD pin mappings that gen9_bc uses.
>
> Changes since v4:
> * Split this into its own commit
> * Introduce skl_hpd_pin() like vsyrjala suggested and use that instead of
> sticking our HPD pin mappings in TGP code
>
> Cc: Matt Roper
> Cc: Jani Nikula
> Cc: Ville Syrjala
> [originally from Tejas's work]
> Signed-off-by: Tejas Upadhyay
> Signed-off-by: Lyude Paul
Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 3c4003605f93..01b171f52694 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3954,6 +3954,14 @@ static enum hpd_pin cnl_hpd_pin(struct
> drm_i915_private *dev_priv,
> return HPD_PORT_A + port - PORT_A;
> }
>
> +static enum hpd_pin skl_hpd_pin(struct drm_i915_private *dev_priv, enum port
> port)
> +{
> + if (HAS_PCH_TGP(dev_priv))
> + return icl_hpd_pin(dev_priv, port);
> +
> + return HPD_PORT_A + port - PORT_A;
> +}
> +
> #define port_tc_name(port) ((port) - PORT_TC1 + '1')
> #define tc_port_name(tc_port) ((tc_port) - TC_PORT_1 + '1')
>
> @@ -4070,6 +4078,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv,
> enum port port)
> encoder->hpd_pin = icl_hpd_pin(dev_priv, port);
> else if (IS_GEN(dev_priv, 10))
> encoder->hpd_pin = cnl_hpd_pin(dev_priv, port);
> + else if (IS_GEN(dev_priv, 9))
> + encoder->hpd_pin = skl_hpd_pin(dev_priv, port);
> else
> encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
>
> --
> 2.29.2
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/gen9_bc: Recognize TGP PCH + CML combos
On Tue, Feb 09, 2021 at 04:28:28PM -0500, Lyude Paul wrote: > Since Intel has introduced the gen9_bc platform, a combination of > Tigerpoint PCHs and CML CPUs, let's recognize such platforms as valid and > avoid WARNing on them. > > Changes since v4: > * Split this into it's own patch - vsyrjala > > Cc: Matt Roper > Cc: Jani Nikula > Cc: Ville Syrjala > [originally from Tejas's work] > Signed-off-by: Tejas Upadhyay > Signed-off-by: Lyude Paul Reviewed-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/intel_pch.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pch.c > b/drivers/gpu/drm/i915/intel_pch.c > index 4813207fc053..7476f0e063c6 100644 > --- a/drivers/gpu/drm/i915/intel_pch.c > +++ b/drivers/gpu/drm/i915/intel_pch.c > @@ -121,7 +121,8 @@ intel_pch_type(const struct drm_i915_private *dev_priv, > unsigned short id) > case INTEL_PCH_TGP2_DEVICE_ID_TYPE: > drm_dbg_kms(&dev_priv->drm, "Found Tiger Lake LP PCH\n"); > drm_WARN_ON(&dev_priv->drm, !IS_TIGERLAKE(dev_priv) && > - !IS_ROCKETLAKE(dev_priv)); > + !IS_ROCKETLAKE(dev_priv) && > + !IS_GEN9_BC(dev_priv)); > return PCH_TGP; > case INTEL_PCH_JSP_DEVICE_ID_TYPE: > case INTEL_PCH_JSP2_DEVICE_ID_TYPE: > -- > 2.29.2 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [RFC v3 07/10] drm/i915/dpcd_bl: Move VESA backlight enabling code closer together
On Fri, Feb 05, 2021 at 06:45:11PM -0500, Lyude Paul wrote:
> No functional changes, just move set_vesa_backlight_enable() closer to it's
> only caller: intel_dp_aux_vesa_enable_backlight().
>
> Signed-off-by: Lyude Paul
Reviewed-by: Rodrigo Vivi
> ---
> .../drm/i915/display/intel_dp_aux_backlight.c | 54 +--
> 1 file changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index f5ae2fb34c1f..431758058aa0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -270,33 +270,6 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector
> *connector, enum pipe pi
> }
>
> /* VESA backlight callbacks */
> -static void set_vesa_backlight_enable(struct intel_connector *connector,
> bool enable)
> -{
> - struct intel_dp *intel_dp = intel_attached_dp(connector);
> - struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> - u8 reg_val = 0;
> -
> - /* Early return when display use other mechanism to enable backlight. */
> - if (!connector->panel.backlight.edp.vesa.aux_enable)
> - return;
> -
> - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
> ®_val) != 1) {
> - drm_dbg_kms(&i915->drm, "Failed to read DPCD register 0x%x\n",
> - DP_EDP_DISPLAY_CONTROL_REGISTER);
> - return;
> - }
> - if (enable)
> - reg_val |= DP_EDP_BACKLIGHT_ENABLE;
> - else
> - reg_val &= ~(DP_EDP_BACKLIGHT_ENABLE);
> -
> - if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
> -reg_val) != 1) {
> - drm_dbg_kms(&i915->drm, "Failed to %s aux backlight\n",
> - enable ? "enable" : "disable");
> - }
> -}
> -
> static bool intel_dp_aux_vesa_backlight_dpcd_mode(struct intel_connector
> *connector)
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> @@ -378,6 +351,33 @@ intel_dp_aux_vesa_set_backlight(const struct
> drm_connector_state *conn_state,
> }
> }
>
> +static void set_vesa_backlight_enable(struct intel_connector *connector,
> bool enable)
> +{
> + struct intel_dp *intel_dp = intel_attached_dp(connector);
> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + u8 reg_val = 0;
> +
> + /* Early return when display use other mechanism to enable backlight. */
> + if (!connector->panel.backlight.edp.vesa.aux_enable)
> + return;
> +
> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
> ®_val) != 1) {
> + drm_dbg_kms(&i915->drm, "Failed to read DPCD register 0x%x\n",
> + DP_EDP_DISPLAY_CONTROL_REGISTER);
> + return;
> + }
> + if (enable)
> + reg_val |= DP_EDP_BACKLIGHT_ENABLE;
> + else
> + reg_val &= ~(DP_EDP_BACKLIGHT_ENABLE);
> +
> + if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
> +reg_val) != 1) {
> + drm_dbg_kms(&i915->drm, "Failed to %s aux backlight\n",
> + enable ? "enable" : "disable");
> + }
> +}
> +
> static void
> intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state,
> const struct drm_connector_state
> *conn_state, u32 level)
> --
> 2.29.2
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v3 06/10] drm/i915/dpcd_bl: Cache some backlight capabilities in intel_panel.backlight
On Fri, Feb 05, 2021 at 06:45:10PM -0500, Lyude Paul wrote:
> Since we're about to be moving this code into shared DRM helpers, we might
> as well start to cache certain backlight capabilities that can be
> determined from the EDP DPCD, and are likely to be relevant to the majority
> of drivers using said helpers. The main purpose of this is just to prevent
> every driver from having to check everything against the eDP DPCD using DP
> macros, which makes the code slightly easier to read (especially since the
> names of some of the eDP capabilities don't exactly match up with what we
> actually need to use them for, like DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT
> for instance).
>
> Signed-off-by: Lyude Paul
Reviewed-by: Rodrigo Vivi
> ---
> .../drm/i915/display/intel_display_types.h| 2 ++
> .../drm/i915/display/intel_dp_aux_backlight.c | 29 ---
> 2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index f4b26e1dbaaf..16824eb3ef93 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -265,6 +265,8 @@ struct intel_panel {
> struct {
> u8 pwmgen_bit_count;
> u8 pwm_freq_pre_divider;
> + bool lsb_reg_used;
> + bool aux_enable;
> } vesa;
> struct {
> bool sdr_uses_aux;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index 95e3e344cf40..f5ae2fb34c1f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -270,13 +270,14 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector
> *connector, enum pipe pi
> }
>
> /* VESA backlight callbacks */
> -static void set_vesa_backlight_enable(struct intel_dp *intel_dp, bool enable)
> +static void set_vesa_backlight_enable(struct intel_connector *connector,
> bool enable)
> {
> + struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> u8 reg_val = 0;
>
> /* Early return when display use other mechanism to enable backlight. */
> - if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP))
> + if (!connector->panel.backlight.edp.vesa.aux_enable)
> return;
>
> if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
> ®_val) != 1) {
> @@ -339,9 +340,11 @@ static u32 intel_dp_aux_vesa_get_backlight(struct
> intel_connector *connector, en
> DP_EDP_BACKLIGHT_BRIGHTNESS_MSB);
> return 0;
> }
> - level = read_val[0];
> - if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
> +
> + if (connector->panel.backlight.edp.vesa.lsb_reg_used)
> level = (read_val[0] << 8 | read_val[1]);
> + else
> + level = read_val[0];
>
> return level;
> }
> @@ -359,13 +362,14 @@ intel_dp_aux_vesa_set_backlight(const struct
> drm_connector_state *conn_state,
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> u8 vals[2] = { 0x0 };
>
> - vals[0] = level;
> -
> /* Write the MSB and/or LSB */
> - if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT) {
> + if (connector->panel.backlight.edp.vesa.lsb_reg_used) {
> vals[0] = (level & 0xFF00) >> 8;
> vals[1] = (level & 0xFF);
> + } else {
> + vals[0] = level;
> }
> +
> if (drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
> vals,
> sizeof(vals)) != sizeof(vals)) {
> drm_dbg_kms(&i915->drm,
> @@ -419,14 +423,13 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
> }
>
> intel_dp_aux_vesa_set_backlight(conn_state, level);
> - set_vesa_backlight_enable(intel_dp, true);
> + set_vesa_backlight_enable(connector, true);
> }
>
> static void intel_dp_aux_vesa_disable_backlight(const struct
> drm_connector_state *old_conn_state,
> u32 level)
> {
> -
> set_vesa_backlight_enable(enc_to_intel_dp(to_intel_encoder(old_conn_state->best_encoder)),
>
Re: [RFC v3 04/10] drm/i915/dpcd_bl: Handle drm_dpcd_read/write() return values correctly
On Fri, Feb 05, 2021 at 06:45:08PM -0500, Lyude Paul wrote:
> This is kind of an annoying aspect of DRM's DP helpers:
> drm_dp_dpcd_readb/writeb() return the size of bytes read/written on
> success, thus we want to check against that instead of checking if the
> return value is less than 0.
>
> I'll probably be fixing this in the near future once I start doing DP work
> again, also because I'd rather not mix a tree-wide refactor like that in
> with a patch series intended to be around introducing DP backlight helpers.
> So, for now let's just handle the return values from each function
> correctly.
>
> Signed-off-by: Lyude Paul
Reviewed-by: Rodrigo Vivi
> ---
> .../drm/i915/display/intel_dp_aux_backlight.c | 41 +--
> 1 file changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index 62294967f430..c37ccc8538cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -107,7 +107,7 @@ intel_dp_aux_supports_hdr_backlight(struct
> intel_connector *connector)
> u8 tcon_cap[4];
>
> ret = drm_dp_dpcd_read(aux, INTEL_EDP_HDR_TCON_CAP0, tcon_cap,
> sizeof(tcon_cap));
> - if (ret < 0)
> + if (ret != sizeof(tcon_cap))
> return false;
>
> if (!(tcon_cap[1] & INTEL_EDP_HDR_TCON_BRIGHTNESS_NITS_CAP))
> @@ -137,7 +137,7 @@ intel_dp_aux_hdr_get_backlight(struct intel_connector
> *connector, enum pipe pipe
> u8 tmp;
> u8 buf[2] = { 0 };
>
> - if (drm_dp_dpcd_readb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS,
> &tmp) < 0) {
> + if (drm_dp_dpcd_readb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS,
> &tmp) != 1) {
> drm_err(&i915->drm, "Failed to read current backlight mode from
> DPCD\n");
> return 0;
> }
> @@ -153,7 +153,8 @@ intel_dp_aux_hdr_get_backlight(struct intel_connector
> *connector, enum pipe pipe
> return panel->backlight.max;
> }
>
> - if (drm_dp_dpcd_read(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB,
> buf, sizeof(buf)) < 0) {
> + if (drm_dp_dpcd_read(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB, buf,
> + sizeof(buf)) != sizeof(buf)) {
> drm_err(&i915->drm, "Failed to read brightness from DPCD\n");
> return 0;
> }
> @@ -172,7 +173,8 @@ intel_dp_aux_hdr_set_aux_backlight(const struct
> drm_connector_state *conn_state,
> buf[0] = level & 0xFF;
> buf[1] = (level & 0xFF00) >> 8;
>
> - if (drm_dp_dpcd_write(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB,
> buf, 4) < 0)
> + if (drm_dp_dpcd_write(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB,
> buf,
> + sizeof(buf)) != sizeof(buf))
> drm_err(dev, "Failed to write brightness level to DPCD\n");
> }
>
> @@ -203,7 +205,7 @@ intel_dp_aux_hdr_enable_backlight(const struct
> intel_crtc_state *crtc_state,
> u8 old_ctrl, ctrl;
>
> ret = drm_dp_dpcd_readb(&intel_dp->aux,
> INTEL_EDP_HDR_GETSET_CTRL_PARAMS, &old_ctrl);
> - if (ret < 0) {
> + if (ret != 1) {
> drm_err(&i915->drm, "Failed to read current backlight control
> mode: %d\n", ret);
> return;
> }
> @@ -221,7 +223,7 @@ intel_dp_aux_hdr_enable_backlight(const struct
> intel_crtc_state *crtc_state,
> }
>
> if (ctrl != old_ctrl)
> - if (drm_dp_dpcd_writeb(&intel_dp->aux,
> INTEL_EDP_HDR_GETSET_CTRL_PARAMS, ctrl) < 0)
> + if (drm_dp_dpcd_writeb(&intel_dp->aux,
> INTEL_EDP_HDR_GETSET_CTRL_PARAMS, ctrl) != 1)
> drm_err(&i915->drm, "Failed to configure DPCD
> brightness controls\n");
> }
>
> @@ -277,8 +279,7 @@ static void set_vesa_backlight_enable(struct intel_dp
> *intel_dp, bool enable)
> if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP))
> return;
>
> - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
> - ®_val) < 0) {
> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
> ®_val) != 1) {
> drm_dbg_kms(&i915->drm, "Failed to read DPCD register 0x%x\n",
> DP_EDP_DISPLAY_CONTROL_REGISTER);
>
Re: [RFC v2 4/5] drm/dp: Extract i915's eDP backlight code into DRM helpers
On Mon, Jan 25, 2021 at 07:10:30PM -0500, Lyude Paul wrote:
> Since we're about to implement eDP backlight support in nouveau using the
> standard protocol from VESA, we might as well just take the code that's
> already written for this and move it into a set of shared DRM helpers.
>
> Note that these helpers are intended to handle DPCD related backlight
> control bits such as setting the brightness level over AUX, probing the
> backlight's TCON, enabling/disabling the backlight over AUX if supported,
> etc. Any PWM-related portions of backlight control are explicitly left up
> to the driver, as these will vary from platform to platform.
>
> The only exception to this is the calculation of the PWM frequency
> pre-divider value. This is because the only platform-specific information
> required for this is the PWM frequency of the panel, which the driver is
> expected to provide if available. The actual algorithm for calculating this
> value is standard and is defined in the eDP specification from VESA.
>
> Note that these helpers do not yet implement the full range of features
> the VESA backlight interface provides, and only provide the following
> functionality (all of which was already present in i915's DPCD backlight
> support):
This is definitely a good move.
Also the functions are well defined and well documented.
I noticed it wouldn't be straightforward, but I was wondering if it is possible
to make the change in 2 steps (at least):
1. modify i915 code in place to match new functions
2. move to drm adding the documentation, proper returns etc
I couldn't get a good sense of the changes around DPCD mode for instance.
>
> * Basic control of brightness levels
> * Basic probing of backlight capabilities
> * Helpers for enabling and disabling the backlight
>
> Signed-off-by: Lyude Paul
> Cc: Jani Nikula
> Cc: Dave Airlie
> Cc: greg.depo...@gmail.com
> ---
> drivers/gpu/drm/drm_dp_helper.c | 332 ++
> .../drm/i915/display/intel_display_types.h| 3 +-
> .../drm/i915/display/intel_dp_aux_backlight.c | 288 ++-
> include/drm/drm_dp_helper.h | 48 +++
> 4 files changed, 413 insertions(+), 258 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index eedbb48815b7..04cb2b6970a8 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -3082,3 +3082,335 @@ int drm_dp_pcon_convert_rgb_to_ycbcr(struct
> drm_dp_aux *aux, u8 color_spc)
> return 0;
> }
> EXPORT_SYMBOL(drm_dp_pcon_convert_rgb_to_ycbcr);
> +
> +/**
> + * drm_edp_backlight_set_level() - Set the backlight level of an eDP panel
> via AUX
> + * @aux: The DP AUX channel to use
> + * @bl: Backlight capability info from drm_edp_backlight_init()
> + * @level: The brightness level to set
> + *
> + * Sets the brightness level of an eDP panel's backlight. Note that the
> panel's backlight must
> + * already have been enabled by the driver by calling
> drm_edp_backlight_enable().
> + *
> + * Returns: %0 on success, negative error code on failure
> + */
> +int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
> + u16 level)
> +{
> + int ret;
> + u8 buf[2] = { 0 };
> +
> + if (bl->lsb_reg_used) {
> + buf[0] = (level & 0xFF00) >> 8;
> + buf[1] = (level & 0x00FF);
> + } else {
> + buf[0] = level;
> + }
> +
> + ret = drm_dp_dpcd_write(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, buf,
> sizeof(buf));
> + if (ret != sizeof(buf)) {
> + DRM_ERROR("%s: Failed to write aux backlight level: %d\n",
> aux->name, ret);
> + return ret < 0 ? ret : -EIO;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_edp_backlight_set_level);
> +
> +static int
> +drm_edp_backlight_set_enable(struct drm_dp_aux *aux, const struct
> drm_edp_backlight_info *bl,
> + bool enable)
> +{
> + int ret;
> + u8 buf;
> +
> + /* The panel uses something other then DPCD for enabling it's backlight
> */
> + if (!bl->aux_enable)
> + return 0;
> +
> + ret = drm_dp_dpcd_readb(aux, DP_EDP_DISPLAY_CONTROL_REGISTER, &buf);
> + if (ret != 1) {
> + DRM_ERROR("%s: Failed to read eDP display control register:
> %d\n", aux->name, ret);
> + return ret < 0 ? ret : -EIO;
> + }
> + if (enable)
> + buf |= DP_EDP_BACKLIGHT_ENABLE;
> + else
> + buf &= ~DP_EDP_BACKLIGHT_ENABLE;
> +
> + ret = drm_dp_dpcd_writeb(aux, DP_EDP_DISPLAY_CONTROL_REGISTER, buf);
> + if (ret != 1) {
> + DRM_ERROR("%s: Failed to write eDP display control register:
> %d\n", aux->name, ret);
> + return ret < 0 ? ret : -EIO;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * drm_edp_backlight_enable() - Enable an eDP panel's backlight using DPCD
> + * @aux: The DP AUX
Re: linux-next: build failure after merge of the drm-intel-fixes tree
On Wed, Nov 04, 2020 at 09:37:05AM +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the drm-intel-fixes tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> drivers/gpu/drm/i915/gt/intel_lrc.c: In function 'gen12_emit_fini_breadcrumb':
> drivers/gpu/drm/i915/gt/intel_lrc.c:4998:31: error: implicit declaration of
> function '__gen8_emit_flush_dw'; did you mean 'gen8_emit_flush'?
> [-Werror=implicit-function-declaration]
> 4998 | cs = emit_xcs_breadcrumb(rq, __gen8_emit_flush_dw(cs, 0, 0, 0));
> | ^~~~
> | gen8_emit_flush
> drivers/gpu/drm/i915/gt/intel_lrc.c:4998:31: warning: passing argument 2 of
> 'emit_xcs_breadcrumb' makes pointer from integer without a cast
> [-Wint-conversion]
> 4998 | cs = emit_xcs_breadcrumb(rq, __gen8_emit_flush_dw(cs, 0, 0, 0));
> | ^
> | |
> | int
> drivers/gpu/drm/i915/gt/intel_lrc.c:4902:63: note: expected 'u32 *' {aka
> 'unsigned int *'} but argument is of type 'int'
> 4902 | static u32 *emit_xcs_breadcrumb(struct i915_request *rq, u32 *cs)
> | ~^~
>
> Caused by commit
>
> c94d65d2ff6d ("drm/i915/gt: Flush xcs before tgl breadcrumbs")
>
> I have reverted that commit for today.
Sorry for the trouble. Dependency picked to drm-intel-fixes now.
Thanks for reporting,
Rodrigo.
>
> --
> Cheers,
> Stephen Rothwell
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RFC v2 3/8] drm/i915: Keep track of pwm-related backlight hooks separately
On Wed, Sep 16, 2020 at 01:18:50PM -0400, Lyude Paul wrote:
> Currently, every different type of backlight hook that i915 supports is
> pretty straight forward - you have a backlight, probably through PWM
> (but maybe DPCD), with a single set of platform-specific hooks that are
> used for controlling it.
>
> HDR backlights, in particular VESA and Intel's HDR backlight
> implementations, can end up being more complicated. With Intel's
> proprietary interface, HDR backlight controls always run through the
> DPCD. When the backlight is in SDR backlight mode however, the driver
> may need to bypass the TCON and control the backlight directly through
> PWM.
>
> So, in order to support this we'll need to split our backlight callbacks
> into two groups: a set of high-level backlight control callbacks in
> intel_panel, and an additional set of pwm-specific backlight control
> callbacks. This also implies a functional changes for how these
> callbacks are used:
>
> * We now keep track of two separate backlight level ranges, one for the
> high-level backlight, and one for the pwm backlight range
> * We also keep track of backlight enablement and PWM backlight
> enablement separately
> * Since the currently set backlight level might not be the same as the
> currently programmed PWM backlight level, we stop setting
> panel->backlight.level with the currently programmed PWM backlight
> level in panel->backlight.pwm_funcs.setup(). Instead, we rely
> on the higher level backlight control functions to retrieve the
> current PWM backlight level (in this case, intel_pwm_get_backlight()).
> Note that there are still a few PWM backlight setup callbacks that
> do actually need to retrieve the current PWM backlight level, although
> we no longer save this value in panel->backlight.level like before.
> * panel->backlight.pwm_funcs.enable()/disable() both accept a PWM
> brightness level, unlike their siblings
> panel->backlight.enable()/disable(). This is so we can calculate the
> actual PWM brightness level we want to set on disable/enable in the
> higher level backlight enable()/disable() functions, since this value
> might be scaled from a brightness level that doesn't come from PWM.
>
> Signed-off-by: Lyude Paul
> Cc: thay...@noraisin.net
> Cc: Vasily Khoruzhick
> ---
> .../drm/i915/display/intel_display_types.h| 14 +-
> drivers/gpu/drm/i915/display/intel_panel.c| 436 ++
> 2 files changed, 246 insertions(+), 204 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index b2d0edacc58c9..52a6543df842a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -221,6 +221,9 @@ struct intel_panel {
> bool alternate_pwm_increment; /* lpt+ */
>
> /* PWM chip */
> + u32 pwm_min;
> + u32 pwm_max;
> + bool pwm_enabled;
> bool util_pin_active_low; /* bxt+ */
> u8 controller; /* bxt+ only */
> struct pwm_device *pwm;
> @@ -229,6 +232,16 @@ struct intel_panel {
> /* DPCD backlight */
> u8 pwmgen_bit_count;
>
> + struct {
> + int (*setup)(struct intel_connector *connector, enum
> pipe pipe);
> + u32 (*get)(struct intel_connector *connector);
> + void (*set)(const struct drm_connector_state
> *conn_state, u32 level);
> + void (*disable)(const struct drm_connector_state
> *conn_state, u32 level);
> + void (*enable)(const struct intel_crtc_state
> *crtc_state,
> +const struct drm_connector_state
> *conn_state, u32 level);
> + u32 (*hz_to_pwm)(struct intel_connector *connector, u32
> hz);
> + } pwm_funcs;
> +
> struct backlight_device *device;
>
> /* Connector and platform specific backlight functions */
> @@ -238,7 +251,6 @@ struct intel_panel {
> void (*disable)(const struct drm_connector_state *conn_state);
> void (*enable)(const struct intel_crtc_state *crtc_state,
> const struct drm_connector_state *conn_state);
> - u32 (*hz_to_pwm)(struct intel_connector *connector, u32 hz);
I see my poor brain getting confused with these 2 levels with very similar
function sets
with same name.
But I currently have no suggestion for better organization or names...
> void (*power)(struct intel_connector *, bool enable);
> } backlight;
> };
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
> b/drivers/gpu/drm/i915/display/intel_panel.c
> index c0e36244bb07d..6d3e9d51d069c 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_pan
Re: [Intel-gfx] [RFC v2 4/8] drm/i915/dp: Rename eDP VESA backlight interface functions
On Wed, Sep 16, 2020 at 01:18:51PM -0400, Lyude Paul wrote:
> Since we're about to add support for a second type of backlight control
> interface over DP AUX (specifically, Intel's proprietary HDR backlight
> controls) let's rename all of the current backlight hooks we have for
> vesa to make it clear that they're specific to the VESA interface and
> not Intel's.
>
> Signed-off-by: Lyude Paul
> Cc: thay...@noraisin.net
> Cc: Vasily Khoruzhick
Reviewed-by: Rodrigo Vivi
> ---
> .../drm/i915/display/intel_dp_aux_backlight.c | 51 ++-
> 1 file changed, 26 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index acbd7eb66cbe3..f601bcbe8ee46 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -25,7 +25,7 @@
> #include "intel_display_types.h"
> #include "intel_dp_aux_backlight.h"
>
> -static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable)
> +static void set_vesa_backlight_enable(struct intel_dp *intel_dp, bool enable)
> {
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> u8 reg_val = 0;
> @@ -56,7 +56,7 @@ static void set_aux_backlight_enable(struct intel_dp
> *intel_dp, bool enable)
> * Read the current backlight value from DPCD register(s) based
> * on if 8-bit(MSB) or 16-bit(MSB and LSB) values are supported
> */
> -static u32 intel_dp_aux_get_backlight(struct intel_connector *connector)
> +static u32 intel_dp_aux_vesa_get_backlight(struct intel_connector *connector)
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> @@ -99,7 +99,8 @@ static u32 intel_dp_aux_get_backlight(struct
> intel_connector *connector)
> * 8-bit or 16 bit value (MSB and LSB)
> */
> static void
> -intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32
> level)
> +intel_dp_aux_vesa_set_backlight(const struct drm_connector_state *conn_state,
> + u32 level)
> {
> struct intel_connector *connector =
> to_intel_connector(conn_state->connector);
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> @@ -129,7 +130,7 @@ intel_dp_aux_set_backlight(const struct
> drm_connector_state *conn_state, u32 lev
> * - Where P = 2^Pn, where Pn is the value programmed by field 4:0 of the
> * EDP_PWMGEN_BIT_COUNT register (DPCD Address 00724h)
> */
> -static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector)
> +static bool intel_dp_aux_vesa_set_pwm_freq(struct intel_connector *connector)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> @@ -165,8 +166,8 @@ static bool intel_dp_aux_set_pwm_freq(struct
> intel_connector *connector)
> return true;
> }
>
> -static void intel_dp_aux_enable_backlight(const struct intel_crtc_state
> *crtc_state,
> - const struct drm_connector_state
> *conn_state)
> +static void intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state
> *crtc_state,
> +const struct drm_connector_state
> *conn_state)
> {
> struct intel_connector *connector =
> to_intel_connector(conn_state->connector);
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> @@ -206,7 +207,7 @@ static void intel_dp_aux_enable_backlight(const struct
> intel_crtc_state *crtc_st
> }
>
> if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP)
> - if (intel_dp_aux_set_pwm_freq(connector))
> + if (intel_dp_aux_vesa_set_pwm_freq(connector))
> new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
>
> if (new_dpcd_buf != dpcd_buf) {
> @@ -217,18 +218,18 @@ static void intel_dp_aux_enable_backlight(const struct
> intel_crtc_state *crtc_st
> }
> }
>
> - intel_dp_aux_set_backlight(conn_state,
> -connector->panel.backlight.level);
> - set_aux_backlight_enable(intel_dp, true);
> + intel_dp_aux_vesa_set_backlight(conn_state,
> + connector->panel.backlight.level);
> + set_vesa_backlight_enable(intel_dp, true);
> }
>
> -static void intel_dp_aux_disable_backlight(const struct drm_connector_state
> *old_conn_state)
> +static void intel_dp_aux_vesa_disable_
Re: [RFC v2 1/8] drm/i915/dp: Program source OUI on eDP panels
On Wed, Sep 16, 2020 at 01:18:48PM -0400, Lyude Paul wrote:
> Since we're about to start adding support for Intel's magic HDR
> backlight interface over DPCD, we need to ensure we're properly
> programming this field so that Intel specific sink services are exposed.
> Otherwise, 0x300-0x3ff will just read zeroes.
>
> We also take care not to reprogram the source OUI if it already matches
> what we expect. This is just to be careful so that we don't accidentally
> take the panel out of any backlight control modes we found it in.
>
> v2:
> * Add careful parameter to intel_edp_init_source_oui() to avoid
> re-writing the source OUI if it's already been set during driver
> initialization
>
> Signed-off-by: Lyude Paul
> Cc: thay...@noraisin.net
> Cc: Vasily Khoruzhick
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 33 +
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4bd10456ad188..7db2b6a3cd52e 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3424,6 +3424,29 @@ void intel_dp_sink_set_decompression_state(struct
> intel_dp *intel_dp,
> enable ? "enable" : "disable");
> }
>
> +static void
> +intel_edp_init_source_oui(struct intel_dp *intel_dp, bool careful)
> +{
> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + u8 oui[] = { 0x00, 0xaa, 0x01 };
> + u8 buf[3] = { 0 };
> +
> + /*
> + * During driver init, we want to be careful and avoid changing the
> source OUI if it's
> + * already set to what we want, so as to avoid clearing any state by
> accident
> + */
> + if (careful) {
my first reaction here is why the problem described on the commit message
doesn't
appear during the init, and setting it to the same shouldn't be a problem... but
yeap, I agree the risk of taking panel down is high... let's move with the
careful approach
Reviewed-by: Rodrigo Vivi
> + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SOURCE_OUI, buf,
> sizeof(buf)) < 0)
> + drm_err(&i915->drm, "Failed to read source OUI\n");
> +
> + if (memcmp(oui, buf, sizeof(oui)) == 0)
> + return;
> + }
> +
> + if (drm_dp_dpcd_write(&intel_dp->aux, DP_SOURCE_OUI, oui, sizeof(oui))
> < 0)
> + drm_err(&i915->drm, "Failed to write source OUI\n");
> +}
> +
> /* If the sink supports it, try to set the power state appropriately */
> void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> {
> @@ -3443,6 +3466,10 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int
> mode)
> } else {
> struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
>
> + /* Write the source OUI as early as possible */
> + if (intel_dp_is_edp(intel_dp))
> + intel_edp_init_source_oui(intel_dp, false);
> +
> /*
>* When turning on, we need to retry for 1ms to give the sink
>* time to wake up.
> @@ -4607,6 +4634,12 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
> if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> intel_dp_get_dsc_sink_cap(intel_dp);
>
> + /*
> + * If needed, program our source OUI so we can make various
> Intel-specific AUX services
> + * available (such as HDR backlight controls)
> + */
> + intel_edp_init_source_oui(intel_dp, true);
> +
> return true;
> }
>
> --
> 2.26.2
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 4/5] drm/i915: Enable Intel's HDR backlight interface (only SDR for now)
On Tue, Sep 15, 2020 at 01:29:38PM -0400, Lyude Paul wrote:
> So-recently a bunch of laptops on the market have started using DPCD
> backlight controls instead of the traditional DDI backlight controls.
> Originally we thought we had this handled by adding VESA backlight
> control support to i915, but the story ended up being a lot more
> complicated then that.
>
> Simply put-there's two main backlight interfaces Intel can see in the
> wild. Intel's proprietary HDR backlight interface, and the standard VESA
> backlight interface. Note that many panels have been observed to report
> support for both backlight interfaces, but testing has shown far more
> panels work with the Intel HDR backlight interface at the moment.
> Additionally, the VBT appears to be capable of reporting support for the
> VESA backlight interface but not the Intel HDR interface which needs to
> be probed by setting the right magic OUI.
>
> On top of that however, there's also actually two different variants of
> the Intel HDR backlight interface. The first uses the AUX channel for
> controlling the brightness of the screen in both SDR and HDR mode, and
> the second only uses the AUX channel for setting the brightness level in
> HDR mode - relying on PWM for setting the brightness level in SDR mode.
>
> For the time being we've been using EDIDs to maintain a list of quirks
> for panels that safely do support the VESA backlight interface. Adding
> support for Intel's HDR backlight interface in addition however, should
> finally allow us to auto-detect eDP backlight controls properly so long
> as we probe like so:
>
> * If the panel's VBT reports VESA backlight support, assume it really
> does support it
> * If the panel's VBT reports DDI backlight controls:
> * First probe for Intel's HDR backlight interface
> * If that fails, probe for VESA's backlight interface
> * If that fails, assume no DPCD backlight control
> * If the panel's VBT reports any other backlight type: just assume it
> doesn't have DPCD backlight controls
>
> Note as well that in order for us to make Intel's HDR backlight
> interface appear, we need to start programming the appropriate source
> OUI on the eDP panel as early as possible in the probing process. Note
> that this technically could be done at any time before setting up
> backlight controls, but this way allows us to avoid re-writing it
> multiple times in case we need to use other source-OUI enabled features
> in the future.
>
> Finally, we also make sure to document the registers for this backlight
> interface since eventually, we want to actually implement the full
> interface instead of keeping it in SDR mode.
>
> Signed-off-by: Lyude Paul
> Cc: thay...@noraisin.net
> Cc: Vasily Khoruzhick
> ---
> .../drm/i915/display/intel_display_types.h| 9 +-
> .../drm/i915/display/intel_dp_aux_backlight.c | 384 +++---
> drivers/gpu/drm/i915/display/intel_panel.c| 34 +-
> drivers/gpu/drm/i915/display/intel_panel.h| 4 +
> drivers/gpu/drm/i915/i915_params.c| 2 +-
> 5 files changed, 381 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 52a6543df842a..9d540368bac89 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -230,7 +230,14 @@ struct intel_panel {
> struct pwm_state pwm_state;
>
> /* DPCD backlight */
> - u8 pwmgen_bit_count;
> + union {
> + struct {
> + u8 pwmgen_bit_count;
> + } vesa;
> + struct {
> + bool sdr_uses_aux;
> + } intel;
> + } edp;
>
> struct {
> int (*setup)(struct intel_connector *connector, enum
> pipe pipe);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index acbd7eb66cbe3..aa1429302db70 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -22,10 +22,251 @@
> *
> */
>
> +/*
> + * Laptops with Intel GPUs which have panels that support controlling the
> + * backlight through DP AUX can actually use two different interfaces:
> Intel's
> + * proprietary DP AUX backlight interface, and the standard VESA backlight
> + * interface. Unfortunately, at the time of writing this a lot of laptops
> will
> + * advertise support for the standard VESA backlight interface when they
> + * don't properly support it. However, on these systems the Intel backlight
> + * interface generally does work properly. Additionally, these systems will
> + * usually just indicate that they use PWM backlight controls in their VBIOS
> + * for some reason.
> + */
> +
>
Re: [RFC 1/5] drm/i915/dp: Program source OUI on eDP panels
On Tue, Sep 15, 2020 at 01:29:35PM -0400, Lyude Paul wrote:
> Since we're about to start adding support for Intel's magic HDR
> backlight interface over DPCD, we need to ensure we're properly
> programming this field so that Intel specific sink services are exposed.
> Otherwise, 0x300-0x3ff will just read zeroes.
>
> We also take care not to reprogram the source OUI if it already matches
> what we expect. This is just to be careful so that we don't accidentally
> take the panel out of any backlight control modes we found it in.
>
> Signed-off-by: Lyude Paul
> Cc: thay...@noraisin.net
> Cc: Vasily Khoruzhick
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 32 +
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4bd10456ad188..b591672ec4eab 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3428,6 +3428,7 @@ void intel_dp_sink_set_decompression_state(struct
> intel_dp *intel_dp,
> void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> {
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + u8 edp_oui[] = { 0x00, 0xaa, 0x01 };
what are these values?
> int ret, i;
>
> /* Should have a valid DPCD by this point */
> @@ -3443,6 +3444,14 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int
> mode)
> } else {
> struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
>
> + /* Write the source OUI as early as possible */
> + if (intel_dp_is_edp(intel_dp)) {
> + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_SOURCE_OUI,
> edp_oui,
> + sizeof(edp_oui));
> + if (ret < 0)
> + drm_err(&i915->drm, "Failed to write eDP source
> OUI\n");
> + }
> +
> /*
>* When turning on, we need to retry for 1ms to give the sink
>* time to wake up.
> @@ -4530,6 +4539,23 @@ static void intel_dp_get_dsc_sink_cap(struct intel_dp
> *intel_dp)
> }
> }
>
> +static void
> +intel_edp_init_source_oui(struct intel_dp *intel_dp)
> +{
> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + u8 oui[] = { 0x00, 0xaa, 0x01 };
> + u8 buf[3] = { 0 };
> +
> + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SOURCE_OUI, buf, sizeof(buf)) <
> 0)
> + drm_err(&i915->drm, "Failed to read source OUI\n");
> +
> + if (memcmp(oui, buf, sizeof(oui)) == 0)
> + return;
> +
> + if (drm_dp_dpcd_write(&intel_dp->aux, DP_SOURCE_OUI, oui, sizeof(oui))
> < 0)
> + drm_err(&i915->drm, "Failed to write source OUI\n");
> +}
> +
> static bool
> intel_edp_init_dpcd(struct intel_dp *intel_dp)
> {
> @@ -4607,6 +4633,12 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
> if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> intel_dp_get_dsc_sink_cap(intel_dp);
>
> + /*
> + * Program our source OUI so we can make various Intel-specific AUX
> + * services available (such as HDR backlight controls)
> + */
> + intel_edp_init_source_oui(intel_dp);
I believe we should restrict this to the supported platforms: cfl, whl, cml,
icl, tgl
no?
> +
> return true;
> }
>
> --
> 2.26.2
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm fixes for 5.7-rc8/final
On Fri, May 29, 2020 at 12:15:27PM +1000, Dave Airlie wrote:
> On Fri, 29 May 2020 at 12:02, Dave Airlie wrote:
> >
> > On Fri, 29 May 2020 at 11:49, Linus Torvalds
> > wrote:
> > >
> > > On Thu, May 28, 2020 at 5:21 PM Dave Airlie wrote:
> > > >
> > > > Seems to have wound down nicely, a couple of i915 fixes, amdgpu fixes
> > > > and minor ingenic fixes.
> > >
> > > Dave, this doesn't even build. WTF?
> > >
> > > In drivers/gpu/drm/i915/gt/selftest_lrc.c, there's a
> > > engine_heartbeat_disable() function that takes two arguments, but the
> > > new "live_timeslice_nopreempt()" gives it only one.
> > >
> > > I'd blame a merge problem, since the failure is in new code, but the
> > > problem exists in your top-of-tree, not just my merge.
> > >
> > > In fact, it's not even your merge of the i915 tree that is the source
> > > of the problem (although the fact that you clearly didn't _test_ the
> > > end result most definitely is _part_ of the problem!).
> > >
> > > Because the problem exists in the commit that introduced that thing:
> > > commit 1f65efb624c4 ("drm/i915/gt: Prevent timeslicing into
> > > unpreemptable requests").
> > >
> > > It's garbage, and never compiled.
> >
> > I thought I'd dropped the ball completely. but I see it's a selftest
> > failure, I must not have selftests built in my config here, I don't do
> > exhaustive builds randconfig
> >
> > This has also been built by Intel, but I'm assuming they missed their
> > selftest bits as well.
> >
> > I'll revert that and resend.
>
> I did drop the ball in one way, I see sfr reported it broken this morning
>
> I normally expect stuff coming from Intel has been through their CI,
> even their fixes tree generally gets pushed through that system before
> I get it, and it usually catches these things.
>
> I might have to push back on intel fixes this late in the day, as
> maybe the land on next and cherry-pick to fixes model has made them a
> bit lax on how much stuff goes through CI.
>
> I've just drop the whole i915 fixes from the tree and will resend with
> it removed.
Yes, that was my fault. I also didn't have the selftest on my drm-intel-fixes
build and I confused the build run numbers when checking the
https://intel-gfx-ci.01.org/tree/drm-intel-fixes/index.html?
before sending...
If we have another round next week I will make sure CI runs well before
sending another pull request.
Sorry,
Rodrigo.
>
> Dave.
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Current LTS and their EOL
Hi Greg, Ben, and all Is https://www.kernel.org/category/releases.html updated in terms of EOL? Some news out of Linaro conference [2] generated a lot of doubts and questions around. Specially because on the way it was stated by the news 3.16 wouldn't be active anymore. So I'm not sure about the news, but I'd like confirmation from you about expected EOL. [2] https://itsfoss.com/linux-lts-kernel-six-years/ Thanks in advance, Rodrigo.
Re: [Intel-gfx] [PATCH] gpu: drm: i915: Change return type to vm_fault_t
On Wed, Apr 18, 2018 at 08:46:44AM +0300, Jani Nikula wrote:
> On Tue, 17 Apr 2018, Souptick Joarder wrote:
> > On 17-Apr-2018 9:45 PM, "Matthew Wilcox" wrote:
> >>
> >> On Tue, Apr 17, 2018 at 09:14:32PM +0530, Souptick Joarder wrote:
> >> > Not exactly. The plan for these patches is to introduce new vm_fault_t
> > type
> >> > in vm_operations_struct fault handlers. It's now available in 4.17-rc1.
> > We will
> >> > push all the required drivers/filesystem changes through different
> > maintainers
> >> > to linus tree. Once everything is converted into vm_fault_t type then
> > Changing
> >> > it from a signed to an unsigned int causes GCC to warn about an
> > assignment
> >> > from an incompatible type -- int foo(void) is incompatible with
> >> > unsigned int foo(void).
> >> >
> >> > Please refer 1c8f422059ae ("mm: change return type to vm_fault_t") in
> > 4.17-rc1.
> >>
> >> I think this patch would be clearer if you did
> >>
> >> - int ret;
> >> + int err;
> >> + vm_fault_t ret;
> >>
> >> Then it would be clearer to the maintainer that you're splitting apart the
> >> VM_FAULT and errno codes.
> >>
> >> Sorry for not catching this during initial review.
> >
> > Ok, I will make required changes and send v2. Sorry, even I missed this :)
>
> I'm afraid Daniel is closer to the truth.
+1.
> My bad, sorry for the noise.
I opened this thread to add exactly question/noise ;).
So my recommendation for some next time is to make the intention clear
on the commit message itself from the begin.
>
> BR,
> Jani.
>
>
>
> >>
> >> > On Tue, Apr 17, 2018 at 8:59 PM, Jani Nikula
> >> > wrote:
> >> > > On Tue, 17 Apr 2018, Souptick Joarder wrote:
> >> > >> Use new return type vm_fault_t for fault handler. For
> >> > >> now, this is just documenting that the function returns
> >> > >> a VM_FAULT value rather than an errno. Once all instances
> >> > >> are converted, vm_fault_t will become a distinct type.
> >> > >>
> >> > >> Reference id -> 1c8f422059ae ("mm: change return type to
> >> > >> vm_fault_t")
> >> > >>
> >> > >> Signed-off-by: Souptick Joarder
> >> > >> ---
> >> > >> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> >> > >> drivers/gpu/drm/i915/i915_gem.c | 15 ---
> >> > >> 2 files changed, 10 insertions(+), 8 deletions(-)
> >> > >>
> >> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> >> > >> index a42deeb..95b0d50 100644
> >> > >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> > >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> > >> @@ -51,6 +51,7 @@
> >> > >> #include
> >> > >> #include
> >> > >> #include
> >> > >> +#include
> >> > >>
> >> > >> #include "i915_params.h"
> >> > >> #include "i915_reg.h"
> >> > >> @@ -3363,7 +3364,7 @@ int i915_gem_wait_for_idle(struct
> > drm_i915_private *dev_priv,
> >> > >> unsigned int flags);
> >> > >> int __must_check i915_gem_suspend(struct drm_i915_private
> > *dev_priv);
> >> > >> void i915_gem_resume(struct drm_i915_private *dev_priv);
> >> > >> -int i915_gem_fault(struct vm_fault *vmf);
> >> > >> +vm_fault_t i915_gem_fault(struct vm_fault *vmf);
> >> > >> int i915_gem_object_wait(struct drm_i915_gem_object *obj,
> >> > >>unsigned int flags,
> >> > >>long timeout,
> >> > >> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> >> > >> index dd89abd..bdac690 100644
> >> > >> --- a/drivers/gpu/drm/i915/i915_gem.c
> >> > >> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> > >> @@ -1882,7 +1882,7 @@ int i915_gem_mmap_gtt_version(void)
> >> > >> * The current feature set supported by i915_gem_fault() and thus
> > GTT mmaps
> >> > >> * is exposed via I915_PARAM_MMAP_GTT_VERSION (see
> > i915_gem_mmap_gtt_version).
> >> > >> */
> >> > >> -int i915_gem_fault(struct vm_fault *vmf)
> >> > >> +vm_fault_t i915_gem_fault(struct vm_fault *vmf)
> >> > >> {
> >> > >> #define MIN_CHUNK_PAGES ((1 << 20) >> PAGE_SHIFT) /* 1 MiB */
> >> > >> struct vm_area_struct *area = vmf->vma;
> >> > >> @@ -1895,6 +1895,7 @@ int i915_gem_fault(struct vm_fault *vmf)
> >> > >> pgoff_t page_offset;
> >> > >> unsigned int flags;
> >> > >> int ret;
> >> > >> + vm_fault_t retval;
> >> > >
> >> > > What's the point of changing the name? An unnecessary change.
> >> > >
> >> > > BR,
> >> > > Jani.
> >> > >
> >> > >>
> >> > >> /* We don't use vmf->pgoff since that has the fake offset */
> >> > >> page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
> >> > >> @@ -2000,7 +2001,7 @@ int i915_gem_fault(struct vm_fault *vmf)
> >> > >>* and so needs to be reported.
> >> > >>*/
> >> > >> if (!i915_terminally_wedged(&dev_priv->gpu_error)) {
> >> > >> - ret = VM_FAULT_SIGBUS;
> >> > >> + retval = VM_FAULT_SIGBUS;
> >> > >> break;
> >> > >> }
> >> > >> case -EAGAI
Re: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for Geminilake
On Tue, Apr 17, 2018 at 12:02:52PM +0300, Jani Nikula wrote: > On Mon, 16 Apr 2018, "Srivatsa, Anusha" wrote: > >>-Original Message- > >>From: Jani Nikula [mailto:jani.nik...@linux.intel.com] > >>Sent: Wednesday, April 11, 2018 5:27 AM > >>To: Ian W MORRISON > >>Cc: Vivi, Rodrigo ; Srivatsa, Anusha > >>; Wajdeczko, Michal > >>; Greg KH ; > >>airl...@linux.ie; joonas.lahti...@linux.intel.com; > >>linux-kernel@vger.kernel.org; > >>sta...@vger.kernel.org; intel-...@lists.freedesktop.org; dri- > >>de...@lists.freedesktop.org > >>Subject: Re: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for > >>Geminilake > >> > >>On Wed, 11 Apr 2018, Ian W MORRISON wrote: > >>> > >>> > > NAK on indiscriminate Cc: stable. There are zero guarantees that > older kernels will work with whatever firmware you throw at them. > > >>> > >>> I included 'Cc: stable' so the patch would get added to the v4.16 and > >>> v4.15 kernels which I have tested with the patch. I found that earlier > >>> kernels didn't support the 'linux-firmware' package required to get > >>> wifi working on Intel's new Gemini Lake NUC. > >> > >>You realize that this patch should have nothing to do with wifi? > >> > >>Rodrigo, Anusha, if you think Cc: stable is appropriate, please indicate > >>the specific > >>versions of stable it is appropriate for. > > > > Hi Jani, > > > > The stable kernel version is 4.12 and beyond. > > It is appropriate to add the CC: stable in my opinion > > Who tested the firmware with v4.12 and later? We only have the CI > results against *current* drm-tip. We don't even know about v4.16. > I understand your concerns, but the problem was that our old process was a bit (lot?) messed and there was the unreliable time until the firmware really lands on linux-firmware.git. So MODULE_FIRMWARE call was only added after firmware was really there on firmware repository but it wasn't about the testing. In other words, the bump version patch was merged after tested, but MODULE_FIRMWARE was left behind because firmware blob took a while to get pulled into linux-firmware.git and we end up forgetting to add it there. In my opinion it should be safe to add the MODULE_FIRMWARE there based on the tests from when the version was bumped. > I'm not going to ack and take responsibility for the stable backports > unless someone actually comes forward with credible Tested-bys. > > BR, > Jani. > > > > > > Anusha > >>BR, > >>Jani. > >> > >>> > > PS. How is this a "RESEND"? I haven't seen this before. > > >>> > >>> It is a 'RESEND' for that very reason. I initially sent the patch to > >>> the same people as a similar patch > >>> (https://patchwork.kernel.org/patch/10143637/) however after realising > >>> this omitted required addresses I added them and resent the patch. > >>> > >>> Best regards, > >>> Ian > >> > >>-- > >>Jani Nikula, Intel Open Source Technology Center > > -- > Jani Nikula, Intel Open Source Technology Center > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: manual merge of the drm tree with Linus' tree
On Mon, Feb 19, 2018 at 10:10:50AM +1100, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the drm tree got a conflict in:
>
> drivers/gpu/drm/i915/intel_breadcrumbs.c
>
> between commit:
>
> 117172c8f9d4 ("drm/i915/breadcrumbs: Ignore unsubmitted signalers")
>
> from Linus' tree and commit:
>
> b7a3f33bd5ab ("drm/i915/breadcrumbs: Drop request reference for the
> signaler thread")
>
> from the drm tree.
>
> These are basically identical for the conflicting section except that
> the former added a line:
>
> GEM_BUG_ON(!i915_gem_request_completed(request));
>
> which I left in.
>
> I fixed it up (see above) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
For this and for the PMU one, I'm really sorry. I believe I should had
mentioned this to Dave when sending pull request for drm-intel-fixes.
I didn't mentioned because for what fixes is concerned this shouldn't
be a problem, but I totally forgot about linux-next. Please accept my
apologies.
Do you use any rerere on linux-next? I wonder if drm-rerere could be used
somehow here to simplify this process of propagating conflicts resolutions
like this.
Thanks,
Rodrigo.
>
> --
> Cheers,
> Stephen Rothwell
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] Graphics on thinkpad x270 after dock/undock works only for the first time (CPU pipe B FIFO underrun)
On Sat, Dec 30, 2017 at 12:53:58PM +, Jiri Kosina wrote: > On Sat, 30 Dec 2017, Jiri Kosina wrote: > > > Seems like disabling RC6 on the kernel command line works this around, and > > I can dock / undock several times in a row with the image always coming > > up properly on the external display. > > > > On the first undock, the WARN_ONCE() below triggers, so I believe each > > undock leaks memory. > > > > [ 38.755084] Failed to release pages: bind_count=1, pages_pin_count=1, > > pin_global=0 > > [ 38.755138] WARNING: CPU: 3 PID: 96 at > > ../drivers/gpu/drm/i915/i915_gem_userptr.c:89 cancel_userptr+0xe5/0xf0 > > [i915] > > OK, I am seeing this warning with current Linus' tree (5aa90a845) even > without any attempt to dock/undock, so it's probably unrelated to external > outputs and it only by coincidence appeared originally at the same time I > docked the machine. > > So there are two separate issues on this machine with latest kernel > (neither of them probably being regression): > > - I have to disable i915 RC6 at the kernel cmdline, otherwise external > (dock) display gets output only randomly (seems like always only on > first dock) Joonas, Chris, time to bring rc6_enable back on next-fixes before we remove this support entirely? > > - the warning, which triggers at not really deterministic time after boot, > but usually rather quickly Jiri, could you please report these issues separately on bugs.freedesktop.org? Are them regressions? Possible bisect? Please attach the dmesg booting with drm.debug=0x1e > > -- > Jiri Kosina > SUSE Labs > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
On Sun, Dec 31, 2017 at 10:34:54PM +, Stefan Brüns wrote:
> The ACK/NACK implementation as found in e.g. the G965 has the falling
> clock edge and the release of the data line after the ACK for the received
> byte happen at the same time.
>
> This is conformant with the I2C specification, which allows a zero hold
> time, see footnote [3]: "A device must internally provide a hold time of
> at least 300 ns for the SDA signal (with respect to the V IH(min) of the
> SCL signal) to bridge the undefined region of the falling edge of SCL."
>
> Some HDMI-to-VGA converters apparently fail to adhere to this requirement
> and latch SDA at the falling clock edge, so instead of an ACK
> sometimes a NACK is read and the slave (i.e. the EDID ROM) ends the
> transfer.
>
> The bitbanging releases the data line for the ACK only 1/4 bit time after
> the falling clock edge, so a slave will see the correct value no matter
> if it samples at the rising or the falling clock edge or in the center.
>
> Fallback to bitbanging is already done for the CRT connector.
>
> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92685
s/Bug:/Bugzilla:
Did we get the confirmation that this also fix the Skylake issue
initially reported?
>
> Signed-off-by: Stefan Brüns
>
> ---
>
> Changes in v2:
> - Fix/enhance commit message, no code changes
>
> drivers/gpu/drm/i915/intel_hdmi.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4dea833f9d1b..847cda4c017c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1573,12 +1573,20 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct edid *edid;
> bool connected = false;
> + struct i2c_adapter *i2c;
>
> intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>
> - edid = drm_get_edid(connector,
> - intel_gmbus_get_adapter(dev_priv,
> - intel_hdmi->ddc_bus));
> + i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
> +
> + edid = drm_get_edid(connector, i2c);
> +
> + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO
> bit-banging\n");
> + intel_gmbus_force_bit(i2c, true);
> + edid = drm_get_edid(connector, i2c);
> + intel_gmbus_force_bit(i2c, false);
> + }
Approach seems fine for this case.
I just wonder what would be the risks of forcing this bit and edid read when
nothing is present on the other end?
>
> intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
>
> --
> 2.15.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] linux-next: build failure after merge of the drm-misc tree
On Wed, Dec 06, 2017 at 01:00:15AM +, Stephen Rothwell wrote:
> Hi all,
Hi Stephen,
I had just written the email for you about this.
Feel free to ignore that one since you already found the solution
and sorry for the delay on warning you.
>
> After merging the drm-misc tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> drivers/gpu/drm/i915/intel_dsi.c: In function
> 'intel_dsi_get_panel_orientation':
> drivers/gpu/drm/i915/intel_dsi.c:1673:13: error: storage size of 'plane'
> isn't known
> enum plane plane;
> ^
>
> Caused by commit
>
> 82daca297506 ("drm/i915: Add "panel orientation" property to the panel
> connector, v6.")
>
> interacting with commit
>
> ed15030d7ab0 ("drm/i915: s/enum plane/enum i9xx_plane_id/")
>
> from the drm-intel tree.
>
> I have applied the following merge fix patch for today.
Yes, that's the right one.
Thanks,
Rodrigo.
>
> From: Stephen Rothwell
> Date: Wed, 6 Dec 2017 11:56:32 +1100
> Subject: [PATCH] drm/i915: fix up for "drm/i915: s/enum plane/enum
> i9xx_plane_id/"
>
> Signed-off-by: Stephen Rothwell
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 1b60df3c14a0..f67d321376e4 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1670,7 +1670,7 @@ static int intel_dsi_get_panel_orientation(struct
> intel_connector *connector)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> int orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> - enum plane plane;
> + enum i9xx_plane_id plane;
> u32 val;
>
> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> --
> 2.15.0
>
> --
> Cheers,
> Stephen Rothwell
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [haswell_crtc_enable] WARNING: CPU: 3 PID: 109 at drivers/gpu/drm/drm_vblank.c:1066 drm_wait_one_vblank+0x18f/0x1a0 [drm]
On Mon, Oct 30, 2017 at 07:10:11PM +, Linus Torvalds wrote: > On Mon, Oct 30, 2017 at 12:00 AM, Fengguang Wu wrote: > > CC intel-gfx. > > Thanks, these are all interesting (even if some of them seem to be > from random kernels). > > Fengguang, is this a new script that you started running? Because I'm > *hoping* it's not that rc6 suddenly seems so flaky, and it's really > that you now have a nice new script that started reporting these > things better, even though many of them may be old? yep, on our side there isn't anything on 4.14-rc6 from i915 that could justify this issues. I hope... Well, on the other hand it would be easier to bisect if this is a 4.14-rc6 thing since we just got one patch for i915 and few patches for gvt for this -rc6. > > This particular one I will have to leave to the intel gfx people to comment > on. why is the bisect hard on this case in particularly? random? Is is reported anywhere where we could have access to full logs? I couldn't find any related open issue on our side. Nothing like this on our CI apparently as well. Other related cases that I saw with vblank time out like this, something else on CPU/GPU had already died before that hence the vblank never recieved. So I'd like to see more logs to have a better idea. Thanks, Rodrigo. > >Linus > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: build failure after merge of the drm-intel-fixes tree
On Wed, Oct 11, 2017 at 08:51:06AM +, Mark Brown wrote:
> On Tue, Oct 10, 2017 at 08:03:00AM +0100, Mark Brown wrote:
> > Hi all,
> >
> > After merging the drm-misc-fixes tree, today's linux-next build
> > (x86_allmodconfig) failed like this:
> >
> > CC [M] drivers/gpu/drm/i915/i915_gem_evict.o
> > drivers/gpu/drm/i915/i915_gem_evict.c: In function
> > ‘i915_gem_evict_for_node’:
> > drivers/gpu/drm/i915/i915_gem_evict.c:318:31: error: implicit declaration
> > of function ‘i915_vma_has_userfault’; did you mean ‘i915_vma_pin_count’?
> > [-Werror=implicit-function-declaration]
> >if (flags & PIN_NONFAULT && i915_vma_has_userfault(vma)) {
> >^~
> >i915_vma_pin_count
> >
> > Caused by commit
> >
> > 72872c99b6dbc ("drm/i915: Check PIN_NONFAULT overlaps in evict_for_node")
> >
> > in the drm-intel-fixes tree. I've used the tree from yesterday.
>
> This is still present today.
It is fixed now. Sorry for the trouble.
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[git pull] drm/i915 fixes for v4.14-rc1
Hi Linus, Since Dave is on paternity leave we are sending drm/i915 fixes for v4.14-rc1 directly to you as he had asked us to do. The most critical ones are the GPU reset fix for gen2-4 and GVT fix for a regression that is blocking gvt init to work on your tree. The rest is general fixes for patches coming from drm-next. Thanks, Rodrigo. The following changes since commit 7846b12fe0b5feab5446d892f41b5140c1419109: Merge branch 'drm-vmwgfx-next' of git://people.freedesktop.org/~syeh/repos_linux into drm-next (2017-08-29 10:38:14 +1000) are available in the git repository at: git://anongit.freedesktop.org/git/drm-intel tags/drm-intel-next-fixes-2017-09-07 for you to fetch changes up to 426ca2cb69cda59f32c251d1f3e111aee8c42814: Merge tag 'gvt-fixes-2017-09-06' of https://github.com/01org/gvt-linux into drm-intel-next-fixes (2017-09-06 13:34:13 -0700) drm/i915 fixes for v4.14-rc1 Chris Wilson (6): drm/i915: Quietly cancel FBC activation if CRTC is turned off before worker drm/i915: Recreate vmapping even when the object is pinned drm/i915: Always wake the device to flush the GTT drm/i915: Ignore duplicate VMA stored within the per-object handle LUT drm/i915: Silence sparse by using gfp_t drm/i915: Re-enable GTT following a device reset Jian Jun Chen (1): drm/i915/gvt: Remove one duplicated MMIO Manasi Navare (1): drm/i915/edp: Increase T12 panel delay to 900 ms to fix DP AUX CH timeouts Rodrigo Vivi (1): Merge tag 'gvt-fixes-2017-09-06' of https://github.com/01org/gvt-linux into drm-intel-next-fixes Ville Syrjälä (7): drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear offset drm/i915: Skip fence alignemnt check for the CCS plane drm/i915: Make i9xx_load_ycbcr_conversion_matrix() static drm/i915: Make i2c lock ops static drm/i915: Fix enum pipe vs. enum transcoder for the PCH transcoder drm/i915: Add __rcu to radix tree slot pointer drm/i915: Annotate user relocs with __user Zhi Wang (1): drm/i915: Fix the missing PPAT cache attributes on CNL drivers/gpu/drm/i915/gvt/handlers.c| 1 - drivers/gpu/drm/i915/i915_cmd_parser.c | 2 +- drivers/gpu/drm/i915/i915_drv.c| 12 +++- drivers/gpu/drm/i915/i915_drv.h| 3 + drivers/gpu/drm/i915/i915_gem.c| 28 +--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 ++- drivers/gpu/drm/i915/i915_gem_gtt.c| 8 +-- drivers/gpu/drm/i915/i915_trace.h | 4 +- drivers/gpu/drm/i915/i915_vma.h| 6 ++ drivers/gpu/drm/i915/intel_color.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 110 + drivers/gpu/drm/i915/intel_dp.c| 2 +- drivers/gpu/drm/i915/intel_fbc.c | 4 +- drivers/gpu/drm/i915/intel_fifo_underrun.c | 20 +++--- drivers/gpu/drm/i915/intel_i2c.c | 2 +- drivers/gpu/drm/i915/intel_sdvo.c | 2 +- 16 files changed, 130 insertions(+), 85 deletions(-)
Re: [PATCH v2] drm/i915: Return correct EDP voltage swing table for 0.85V
On Fri, Aug 11, 2017 at 11:32 AM, Matthias Kaehlcke wrote:
> El Mon, Jul 17, 2017 at 12:58:54PM -0700 Matthias Kaehlcke ha dit:
>
>> For 0.85V cnl_get_buf_trans_edp() returns the DP table, instead of EDP.
>> Use the correct table.
>>
>> The error was pointed out by this clang warning:
>>
>> drivers/gpu/drm/i915/intel_ddi.c:392:39: warning: variable
>> 'cnl_ddi_translations_edp_0_85V' is not needed and will not be emitted
>> [-Wunneeded-internal-declaration]
>> static const struct cnl_ddi_buf_trans cnl_ddi_translations_edp_0_85V[] =
>> {
>>
>> Fixes: cf54ca8bc567 ("drm/i915/cnl: Implement voltage swing sequence.")
>> Signed-off-by: Matthias Kaehlcke
>> Reviewed-by: Manasi Navare
>> ---
>> Changes in v2:
>> - Added 'Fixes' tag
>> - Added Reviewed-by: Manasi Navare
>
> ping, it seems this patch went under the radar.
yes totally. sorry about that.
merged to dinq. Thanks for patch, review, and heads up
> _______
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
Re: [PATCH 04/10] drm: make drm_vblank_{get,put}() static
Oh, nevermind... I saw the places that depends on changes on other
legacy usage like drm_wait_on_vblank... (not trivial on intel_crt)
and other cases...
So better to just go with the static for now.
Feel free to use:
Reviewed-by: Rodrigo Vivi
On Wed, Aug 3, 2016 at 12:22 AM, Daniel Vetter wrote:
> On Tue, Aug 02, 2016 at 11:30:21PM -0700, Rodrigo Vivi wrote:
>> I was going to remove the legacy get/put versions right now, but
>> decided to check if there were any pending patch in mailing lists and
>> found this.
>>
>> What about deleting the functions at all instead of having it internally?
>
> There's (very few) users left, but if you can convert them over to
> drm_crtc_ versions then sure, go ahead, it'd be great!
> -Daniel
>
>>
>>
>> On Tue, Jun 7, 2016 at 7:07 AM, Gustavo Padovan wrote:
>> > From: Gustavo Padovan
>> >
>> > As they are not used anywhere outside drm_irq.c make them static.
>> >
>> > Signed-off-by: Gustavo Padovan
>> > ---
>> > drivers/gpu/drm/drm_irq.c | 10 ++
>> > include/drm/drmP.h| 2 --
>> > 2 files changed, 2 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> > index 38cc782..76e39c5 100644
>> > --- a/drivers/gpu/drm/drm_irq.c
>> > +++ b/drivers/gpu/drm/drm_irq.c
>> > @@ -1108,7 +1108,7 @@ static int drm_vblank_enable(struct drm_device *dev,
>> > unsigned int pipe)
>> > * Returns:
>> > * Zero on success or a negative error code on failure.
>> > */
>> > -int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
>> > +static int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
>> > {
>> > struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>> > unsigned long irqflags;
>> > @@ -1134,7 +1134,6 @@ int drm_vblank_get(struct drm_device *dev, unsigned
>> > int pipe)
>> >
>> > return ret;
>> > }
>> > -EXPORT_SYMBOL(drm_vblank_get);
>> >
>> > /**
>> > * drm_crtc_vblank_get - get a reference count on vblank events
>> > @@ -1143,8 +1142,6 @@ EXPORT_SYMBOL(drm_vblank_get);
>> > * Acquire a reference count on vblank events to avoid having them
>> > disabled
>> > * while in use.
>> > *
>> > - * This is the native kms version of drm_vblank_get().
>> > - *
>> > * Returns:
>> > * Zero on success or a negative error code on failure.
>> > */
>> > @@ -1164,7 +1161,7 @@ EXPORT_SYMBOL(drm_crtc_vblank_get);
>> > *
>> > * This is the legacy version of drm_crtc_vblank_put().
>> > */
>> > -void drm_vblank_put(struct drm_device *dev, unsigned int pipe)
>> > +static void drm_vblank_put(struct drm_device *dev, unsigned int pipe)
>> > {
>> > struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>> >
>> > @@ -1185,7 +1182,6 @@ void drm_vblank_put(struct drm_device *dev, unsigned
>> > int pipe)
>> > jiffies + ((drm_vblank_offdelay *
>> > HZ)/1000));
>> > }
>> > }
>> > -EXPORT_SYMBOL(drm_vblank_put);
>> >
>> > /**
>> > * drm_crtc_vblank_put - give up ownership of vblank events
>> > @@ -1193,8 +1189,6 @@ EXPORT_SYMBOL(drm_vblank_put);
>> > *
>> > * Release ownership of a given vblank counter, turning off interrupts
>> > * if possible. Disable interrupts after drm_vblank_offdelay milliseconds.
>> > - *
>> > - * This is the native kms version of drm_vblank_put().
>> > */
>> > void drm_crtc_vblank_put(struct drm_crtc *crtc)
>> > {
>> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
>> > index 924b4fd..23f79a5 100644
>> > --- a/include/drm/drmP.h
>> > +++ b/include/drm/drmP.h
>> > @@ -975,8 +975,6 @@ extern void drm_crtc_arm_vblank_event(struct drm_crtc
>> > *crtc,
>> > struct drm_pending_vblank_event *e);
>> > extern bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
>> > extern bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
>> > -extern int drm_vblank_get(struct drm_device *dev, unsigned int pipe);
>> > -extern void drm_vblank_put(struct drm_device *dev, unsigned int pipe);
>> > extern int drm_crtc_vblank_get(struct drm_crtc *crtc);
>> > extern void drm_crtc_vblank_put(struct drm_crtc *crtc);
>> > extern void drm_wait_one_vblank(struct drm_device *dev, unsigned int
>> > pipe);
>> > --
>> > 2.5.5
>> >
>> > ___
>> > dri-devel mailing list
>> > dri-de...@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>>
>>
>> --
>> Rodrigo Vivi
>> Blog: http://blog.vivi.eng.br
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
Re: [PATCH 04/10] drm: make drm_vblank_{get,put}() static
I was going to remove the legacy get/put versions right now, but
decided to check if there were any pending patch in mailing lists and
found this.
What about deleting the functions at all instead of having it internally?
On Tue, Jun 7, 2016 at 7:07 AM, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> As they are not used anywhere outside drm_irq.c make them static.
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/gpu/drm/drm_irq.c | 10 ++
> include/drm/drmP.h| 2 --
> 2 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 38cc782..76e39c5 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1108,7 +1108,7 @@ static int drm_vblank_enable(struct drm_device *dev,
> unsigned int pipe)
> * Returns:
> * Zero on success or a negative error code on failure.
> */
> -int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
> +static int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
> {
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> unsigned long irqflags;
> @@ -1134,7 +1134,6 @@ int drm_vblank_get(struct drm_device *dev, unsigned int
> pipe)
>
> return ret;
> }
> -EXPORT_SYMBOL(drm_vblank_get);
>
> /**
> * drm_crtc_vblank_get - get a reference count on vblank events
> @@ -1143,8 +1142,6 @@ EXPORT_SYMBOL(drm_vblank_get);
> * Acquire a reference count on vblank events to avoid having them disabled
> * while in use.
> *
> - * This is the native kms version of drm_vblank_get().
> - *
> * Returns:
> * Zero on success or a negative error code on failure.
> */
> @@ -1164,7 +1161,7 @@ EXPORT_SYMBOL(drm_crtc_vblank_get);
> *
> * This is the legacy version of drm_crtc_vblank_put().
> */
> -void drm_vblank_put(struct drm_device *dev, unsigned int pipe)
> +static void drm_vblank_put(struct drm_device *dev, unsigned int pipe)
> {
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>
> @@ -1185,7 +1182,6 @@ void drm_vblank_put(struct drm_device *dev, unsigned
> int pipe)
> jiffies + ((drm_vblank_offdelay *
> HZ)/1000));
> }
> }
> -EXPORT_SYMBOL(drm_vblank_put);
>
> /**
> * drm_crtc_vblank_put - give up ownership of vblank events
> @@ -1193,8 +1189,6 @@ EXPORT_SYMBOL(drm_vblank_put);
> *
> * Release ownership of a given vblank counter, turning off interrupts
> * if possible. Disable interrupts after drm_vblank_offdelay milliseconds.
> - *
> - * This is the native kms version of drm_vblank_put().
> */
> void drm_crtc_vblank_put(struct drm_crtc *crtc)
> {
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 924b4fd..23f79a5 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -975,8 +975,6 @@ extern void drm_crtc_arm_vblank_event(struct drm_crtc
> *crtc,
> struct drm_pending_vblank_event *e);
> extern bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
> extern bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
> -extern int drm_vblank_get(struct drm_device *dev, unsigned int pipe);
> -extern void drm_vblank_put(struct drm_device *dev, unsigned int pipe);
> extern int drm_crtc_vblank_get(struct drm_crtc *crtc);
> extern void drm_crtc_vblank_put(struct drm_crtc *crtc);
> extern void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe);
> --
> 2.5.5
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
Re: [PATCH] i915: Use 120MHz LVDS SSC clock for gen5/gen6/gen7
On Thu, Nov 14, 2013 at 09:36:10AM -0800, Olof Johansson wrote: > On Thu, Nov 14, 2013 at 8:53 AM, Rodrigo Vivi wrote: > > On Wed, Nov 13, 2013 at 05:59:43PM -0800, Olof Johansson wrote: > >> From: Duncan Laurie > >> > >> We had been using a DMI table workaround to select the right > >> frequency for devices, but this is fragile and must be updated > >> with every new platform. > >> > >> Instead the default case when VBT is missing is changed to use > >> 120MHz clock for LVDS SSC for these generations. > >> > >> The docs for 2010-Core, SandyBridge, and IvyBridge all indicate > >> that the reference frequency for LVDS is 120MHz: > >> > >> "2010 Core" > >> http://intellinuxgraphics.org/IHD_OS_Vol3_Part3r2.pdf > >> page 38 > >> Reference Frequency: 120MHz for CRT and LVDS. 100MHz for the FDI. > >> > >> "2011 SandyBridge" > >> http://intellinuxgraphics.org/documentation/SNB/IHD_OS_Vol3_Part3.pdf > >> page 33 > >> Reference Frequency: 120MHz for CRT, HDMI, LVDS. 100MHz for the FDI. > >> > >> "2012 IvyBridge" > >> http://intellinuxgraphics.org/documentation/IVB/IHD_OS_Vol3_Part4.pdf > >> page 27 > >> Reference Frequency: 120 MHz for CRT, HDMI, LVDS, 100MHz for the FDI. > > > > Checked. You are right. > > And actually true even for HSW and BDW. 120 is the default and 100 is for > > test mode. > > Yeah, the patch predates HSW/BDW, so it was not mentioned. > > >> Signed-off-by: Duncan Laurie > >> [olof: Fixup for recent base, switched from if/else to single call] > >> Signed-off-by: Olof Johansson > >> --- > >> > >> Daniel, > >> > >> This applies on top of -next, which I'm presuming is close to your > >> for-3.13 base right now. It'd be good to see this go in since it's needed > >> to boot on Chromebooks (with developer mode off), and is thus blocking > >> testing next/mainline on a regular basis here. > >> > >> Thanks! > >> > >> -Olof > >> > >> drivers/gpu/drm/i915/intel_bios.c | 7 ++- > >> 1 file changed, 6 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_bios.c > >> b/drivers/gpu/drm/i915/intel_bios.c > >> index 6dd622d733b9..e4fba39631a5 100644 > >> --- a/drivers/gpu/drm/i915/intel_bios.c > >> +++ b/drivers/gpu/drm/i915/intel_bios.c > >> @@ -790,7 +790,12 @@ init_vbt_defaults(struct drm_i915_private *dev_priv) > >> > >> /* Default to using SSC */ > >> dev_priv->vbt.lvds_use_ssc = 1; > >> - dev_priv->vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1); > >> + /* > >> + * Core/SandyBridge/IvyBridge use alternative (120MHz) reference > >> + * clock for LVDS. > >> + */ > >> + dev_priv->vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev, > >> + !HAS_PCH_SPLIT(dev)); > > > > I'm just not convinced this is the right way to fix this here. > > Mainly because for most of platforms the alternate is 100 and default is > > 120. > > The ideal in my opinion should be to invert the alternate inside > > ssc_freqeuncy function and use the exception, that is probably > > IS_PINEVIEW(dev)... > > > > Not sure though... just a guess since this alternate was implemented for > > pineview... > > Seeing the history of some of this code (changes, followed by reverts, > etc), I wanted to stay conservative with what we know works from a few > years in the field by now. Stay conservative is good, but what I don't like is just the inverted logic... or maybe just the "alternate" name. > The vbt defaults are only used by > Chromebooks, as far as I know, so it's not a code path shared with > other platforms today. Are you sure? I don't know who is really setting vbt or going to defaults, but it is possible to have any one using it right now... anyway another reason to stay conservative... > Also, the ssc_freq bit from the vbios table is > passed into this function, so I don't think there's much point in > reversing the logic in there just for one of the two code paths. yeah, maybe just the name alternate is killing me here... > > Finally, from elsewhere in the same file: > > /* > * The genX designation typically refers to the render engine, so render > * capability related checks should use IS_GEN, while display and other checks > * have thei
Re: [PATCH] i915: Use 120MHz LVDS SSC clock for gen5/gen6/gen7
On Wed, Nov 13, 2013 at 05:59:43PM -0800, Olof Johansson wrote:
> From: Duncan Laurie
>
> We had been using a DMI table workaround to select the right
> frequency for devices, but this is fragile and must be updated
> with every new platform.
>
> Instead the default case when VBT is missing is changed to use
> 120MHz clock for LVDS SSC for these generations.
>
> The docs for 2010-Core, SandyBridge, and IvyBridge all indicate
> that the reference frequency for LVDS is 120MHz:
>
> "2010 Core"
> http://intellinuxgraphics.org/IHD_OS_Vol3_Part3r2.pdf
> page 38
> Reference Frequency: 120MHz for CRT and LVDS. 100MHz for the FDI.
>
> "2011 SandyBridge"
> http://intellinuxgraphics.org/documentation/SNB/IHD_OS_Vol3_Part3.pdf
> page 33
> Reference Frequency: 120MHz for CRT, HDMI, LVDS. 100MHz for the FDI.
>
> "2012 IvyBridge"
> http://intellinuxgraphics.org/documentation/IVB/IHD_OS_Vol3_Part4.pdf
> page 27
> Reference Frequency: 120 MHz for CRT, HDMI, LVDS, 100MHz for the FDI.
Checked. You are right.
And actually true even for HSW and BDW. 120 is the default and 100 is for test
mode.
>
> Signed-off-by: Duncan Laurie
> [olof: Fixup for recent base, switched from if/else to single call]
> Signed-off-by: Olof Johansson
> ---
>
> Daniel,
>
> This applies on top of -next, which I'm presuming is close to your
> for-3.13 base right now. It'd be good to see this go in since it's needed
> to boot on Chromebooks (with developer mode off), and is thus blocking
> testing next/mainline on a regular basis here.
>
> Thanks!
>
> -Olof
>
> drivers/gpu/drm/i915/intel_bios.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 6dd622d733b9..e4fba39631a5 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -790,7 +790,12 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
>
> /* Default to using SSC */
> dev_priv->vbt.lvds_use_ssc = 1;
> - dev_priv->vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1);
> + /*
> + * Core/SandyBridge/IvyBridge use alternative (120MHz) reference
> + * clock for LVDS.
> + */
> + dev_priv->vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev,
> + !HAS_PCH_SPLIT(dev));
I'm just not convinced this is the right way to fix this here.
Mainly because for most of platforms the alternate is 100 and default is 120.
The ideal in my opinion should be to invert the alternate inside ssc_freqeuncy
function and use the exception, that is probably IS_PINEVIEW(dev)...
Not sure though... just a guess since this alternate was implemented for
pineview...
> DRM_DEBUG_KMS("Set default to SSC at %dMHz\n",
> dev_priv->vbt.lvds_ssc_freq);
>
> for (port = PORT_A; port < I915_MAX_PORTS; port++) {
> --
> 1.8.4.1.601.g02b3b1d
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
