From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1537C433DB for ; Wed, 3 Feb 2021 21:31:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4739364F2C for ; Wed, 3 Feb 2021 21:31:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232085AbhBCVb2 (ORCPT ); Wed, 3 Feb 2021 16:31:28 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:59089 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229783AbhBCVbT (ORCPT ); Wed, 3 Feb 2021 16:31:19 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612387788; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BfCih1GO7i8fZxSsYoQmI5pUeHjlgZ2OObzYL2IXtSA=; b=CMme71Fdz1j3+z5+v8CVAhkFz50Q294yYI7Xak+ybl37tVxFeZfKDXvTMKwFu1qhGVDpXK o8KOt3h58P2+zo3/L8HrR8s1Q1QVQn10jD9xtu7IRWbr3et0PnmPvuIKRuoyt5dPxhZuJO trzGbAN0KO1u1rxNK2WGEZLo3vjKtTc= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-346-YsrWHJySMvOA4VXnEO7QaA-1; Wed, 03 Feb 2021 16:29:44 -0500 X-MC-Unique: YsrWHJySMvOA4VXnEO7QaA-1 Received: by mail-qv1-f70.google.com with SMTP id l3so523959qvz.12 for ; Wed, 03 Feb 2021 13:29:44 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:reply-to:to:cc:date :in-reply-to:references:organization:user-agent:mime-version :content-transfer-encoding; bh=BfCih1GO7i8fZxSsYoQmI5pUeHjlgZ2OObzYL2IXtSA=; b=EvuU5aVoDRbrqNq2feJQnRApmQvEpula0UAWXw7RTbCpxsnacAF2Q+xtqHe09wUcPr hJc62INL18o2nBXp2PmyzomvA+/7nWmuHBMOrVibAqy/ZKPPx1WYVi7A432ew3uKDGmh p47FnyQt4cFAnysHe1nH+xzPmzEfgO6aWtE5j4MwfB47Du1Zo9BoljO7NaxUkCiXL8LF ZNWbJW5/OFcTi1PEnzi/LqopdCc4Fl5ktB1E+F2b7PD9LzIYQRrjQjiLV0n0f6vXU+tU Vfj7gnFKH7MXhU5NoiOq8fEVb9q0dmTFEkTmG0XxdfeqwTD15pswaRFCZZohrS2y2x0V ZF3A== X-Gm-Message-State: AOAM530jTz06hiX5RVlQnKBNkJwkWos5ISJQ2DTct2P/PQRdsad9whka FB8jaRa6MrzNs3UOffK//sgMUtcoO7WlQp9bML54tIOqDMIzloQy6cazNhYA2m3GGwEkn1SHM4q 5lDTQoLjpk5yd51/+wNxZKkht X-Received: by 2002:a37:9281:: with SMTP id u123mr4578690qkd.77.1612387782450; Wed, 03 Feb 2021 13:29:42 -0800 (PST) X-Google-Smtp-Source: ABdhPJwVoeKFQBViYTpqbKdPw6kwTxovFPrxdm0P08MYH6+Pk8hzkoKCnfbO9EEaIpCCdt4coq557A== X-Received: by 2002:a37:9281:: with SMTP id u123mr4578655qkd.77.1612387782043; Wed, 03 Feb 2021 13:29:42 -0800 (PST) Received: from Whitewolf.lyude.net (pool-108-49-102-102.bstnma.fios.verizon.net. [108.49.102.102]) by smtp.gmail.com with ESMTPSA id k14sm2445402qtj.40.2021.02.03.13.29.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 Feb 2021 13:29:41 -0800 (PST) Message-ID: Subject: Re: [RFC v2 4/5] drm/dp: Extract i915's eDP backlight code into DRM helpers From: Lyude Paul Reply-To: lyude@redhat.com To: Rodrigo Vivi Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Lucas De Marchi , Anshuman Gupta , Jani Nikula , David Airlie , open list , greg.depoire@gmail.com, Uma Shankar , Sean Paul , Thomas Zimmermann , Aaron Ma , Dave Airlie Date: Wed, 03 Feb 2021 16:29:39 -0500 In-Reply-To: <20210203205854.GA302863@intel.com> References: <20210126001031.881048-1-lyude@redhat.com> <20210126001031.881048-5-lyude@redhat.com> <20210203205854.GA302863@intel.com> Organization: Red Hat Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3 (3.38.3-1.fc33) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2021-02-03 at 15:58 -0500, Rodrigo Vivi wrote: > 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. Good point - yeah I can try to do that ASAP > > > > > * 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.depoire@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 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 > > DPCD, while additionally > > + * restoring any important backlight state such as the given backlight > > level, the brightness byte > > + * count, backlight frequency, etc. > > + * > > + * Note that certain panels, while supporting brightness level controls > > over DPCD, may not support > > + * having their backlights enabled via the standard > > %DP_EDP_DISPLAY_CONTROL_REGISTER. On such panels > > + * &drm_edp_backlight_info.aux_enable will be set to %false, this function > > will skip the step of > > + * programming the %DP_EDP_DISPLAY_CONTROL_REGISTER, and the driver must > > perform the required > > + * implementation specific step for enabling the backlight after calling > > this function. > > + * > > + * Returns: %0 on success, negative error code on failure. > > + */ > > +int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct > > drm_edp_backlight_info *bl, > > +                            const u16 level) > > +{ > > +       int ret; > > +       u8 dpcd_buf, new_dpcd_buf; > > + > > +       ret = drm_dp_dpcd_readb(aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, > > &dpcd_buf); > > +       if (ret != 1) { > > +               DRM_DEBUG_KMS("%s: Failed to read backlight mode: %d\n", > > aux->name, ret); > > +               return ret < 0 ? ret : -EIO; > > +       } > > + > > +       new_dpcd_buf = dpcd_buf; > > + > > +       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; > > + > > +               ret = drm_dp_dpcd_writeb(aux, DP_EDP_PWMGEN_BIT_COUNT, bl- > > >pwmgen_bit_count); > > +               if (ret != 1) > > +                       DRM_DEBUG_KMS("%s: Failed to write aux pwmgen bit > > count: %d\n", > > +                                     aux->name, ret); > > +       } > > + > > +       if (bl->pwm_freq_pre_divider) { > > +               ret = drm_dp_dpcd_writeb(aux, DP_EDP_BACKLIGHT_FREQ_SET, bl- > > >pwm_freq_pre_divider); > > +               if (ret != 1) > > +                       DRM_DEBUG_KMS("%s: Failed to write aux backlight > > frequency: %d\n", > > +                                     aux->name, ret); > > +               else > > +                       new_dpcd_buf |= > > DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; > > +       } > > + > > +       if (new_dpcd_buf != dpcd_buf) { > > +               ret = drm_dp_dpcd_writeb(aux, > > DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf); > > +               if (ret != 1) { > > +                       DRM_DEBUG_KMS("%s: Failed to write aux backlight > > mode: %d\n", > > +                                     aux->name, ret); > > +                       return ret < 0 ? ret : -EIO; > > +               } > > +       } > > + > > +       ret = drm_edp_backlight_set_level(aux, bl, level); > > +       if (ret < 0) > > +               return ret; > > +       ret = drm_edp_backlight_set_enable(aux, bl, true); > > +       if (ret < 0) > > +               return ret; > > + > > +       return 0; > > +} > > +EXPORT_SYMBOL(drm_edp_backlight_enable); > > + > > +/** > > + * drm_edp_backlight_disable() - Disable an eDP backlight using DPCD, if > > supported > > + * @aux: The DP AUX channel to use > > + * @bl: Backlight capability info from drm_edp_backlight_init() > > + * > > + * This function handles disabling DPCD backlight controls on a panel over > > AUX. Note that some > > + * panels have backlights that are enabled/disabled by other means, despite > > having their brightness > > + * values controlled through DPCD. On such panels > > &drm_edp_backlight_info.aux_enable will be set to > > + * %false, this function will become a no-op (and we will skip updating > > + * %DP_EDP_DISPLAY_CONTROL_REGISTER), and the driver must take care to > > perform it's own > > + * implementation specific step for disabling the backlight. > > + * > > + * Returns: %0 on success or no-op, negative error code on failure. > > + */ > > +int drm_edp_backlight_disable(struct drm_dp_aux *aux, const struct > > drm_edp_backlight_info *bl) > > +{ > > +       int ret; > > + > > +       ret = drm_edp_backlight_set_enable(aux, bl, false); > > +       if (ret < 0) > > +               return ret; > > + > > +       return 0; > > +} > > +EXPORT_SYMBOL(drm_edp_backlight_disable); > > + > > +static inline int > > +drm_edp_backlight_probe_max(struct drm_dp_aux *aux, struct > > drm_edp_backlight_info *bl, > > +                           u16 driver_pwm_freq_hz, const u8 > > edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE]) > > +{ > > +       int fxp, fxp_min, fxp_max, fxp_actual, f = 1; > > +       int ret; > > +       u8 pn, pn_min, pn_max; > > + > > +       ret = drm_dp_dpcd_readb(aux, DP_EDP_PWMGEN_BIT_COUNT, &pn); > > +       if (ret != 1) { > > +               DRM_DEBUG_KMS("%s: Failed to read pwmgen bit count cap: > > %d\n", aux->name, ret); > > +               return -ENODEV; > > +       } > > + > > +       pn &= DP_EDP_PWMGEN_BIT_COUNT_MASK; > > +       bl->max = (1 << pn) - 1; > > +       if (!driver_pwm_freq_hz) > > +               return 0; > > + > > +       /* > > +        * Set PWM Frequency divider to match desired frequency provided by > > the driver. > > +        * The PWM Frequency is calculated as 27Mhz / (F x P). > > +        * - Where F = PWM Frequency Pre-Divider value programmed by field > > 7:0 of the > > +        *             EDP_BACKLIGHT_FREQ_SET register (DPCD Address 00728h) > > +        * - Where P = 2^Pn, where Pn is the value programmed by field 4:0 > > of the > > +        *             EDP_PWMGEN_BIT_COUNT register (DPCD Address 00724h) > > +        */ > > + > > +       /* 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. > > +        */ > > +       fxp = DIV_ROUND_CLOSEST(1000 * DP_EDP_BACKLIGHT_FREQ_BASE_KHZ, > > driver_pwm_freq_hz); > > + > > +       /* Use highest possible value of Pn for more granularity of > > brightness adjustment while > > +        * satifying the conditions below. > > +        * - Pn is in the range of Pn_min and Pn_max > > +        * - F is in the range of 1 and 255 > > +        * - FxP is within 25% of desired value. > > +        *   Note: 25% is arbitrary value and may need some tweak. > > +        */ > > +       ret = drm_dp_dpcd_readb(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, > > &pn_min); > > +       if (ret != 1) { > > +               DRM_DEBUG_KMS("%s: Failed to read pwmgen bit count cap min: > > %d\n", aux->name, ret); > > +               return 0; > > +       } > > +       ret = drm_dp_dpcd_readb(aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, > > &pn_max); > > +       if (ret != 1) { > > +               DRM_DEBUG_KMS("%s: Failed to read pwmgen bit count cap max: > > %d\n", aux->name, ret); > > +               return 0; > > +       } > > +       pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK; > > +       pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK; > > + > > +       /* Ensure frequency is within 25% of desired value */ > > +       fxp_min = DIV_ROUND_CLOSEST(fxp * 3, 4); > > +       fxp_max = DIV_ROUND_CLOSEST(fxp * 5, 4); > > +       if (fxp_min < (1 << pn_min) || (255 << pn_max) < fxp_max) { > > +               DRM_DEBUG_KMS("%s: Driver defined backlight frequency (%d) > > out of range\n", > > +                             aux->name, driver_pwm_freq_hz); > > +               return 0; > > +       } > > + > > +       for (pn = pn_max; pn >= pn_min; pn--) { > > +               f = clamp(DIV_ROUND_CLOSEST(fxp, 1 << pn), 1, 255); > > +               fxp_actual = f << pn; > > +               if (fxp_min <= fxp_actual && fxp_actual <= fxp_max) > > +                       break; > > +       } > > + > > +       ret = drm_dp_dpcd_writeb(aux, DP_EDP_PWMGEN_BIT_COUNT, pn); > > +       if (ret != 1) { > > +               DRM_DEBUG_KMS("%s: Failed to write aux pwmgen bit count: > > %d\n", aux->name, ret); > > +               return 0; > > +       } > > +       bl->pwmgen_bit_count = pn; > > +       bl->max = (1 << pn) - 1; > > + > > +       if (edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP) { > > +               bl->pwm_freq_pre_divider = f; > > +               DRM_DEBUG_KMS("%s: Using backlight frequency from driver > > (%dHz)\n", > > +                             aux->name, driver_pwm_freq_hz); > > +       } > > + > > +       return 0; > > +} > > + > > +static inline int > > +drm_edp_backlight_probe_level(struct drm_dp_aux *aux, struct > > drm_edp_backlight_info *bl, > > +                             u8 *current_mode) > > +{ > > +       int ret; > > +       u8 buf[2]; > > +       u8 mode_reg; > > + > > +       ret = drm_dp_dpcd_readb(aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, > > &mode_reg); > > +       if (ret != 1) { > > +               DRM_DEBUG_KMS("%s: Failed to read backlight mode: %d\n", > > aux->name, ret); > > +               return ret < 0 ? ret : -EIO; > > +       } > > + > > +       *current_mode = (mode_reg & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK); > > +       if (*current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) { > > +               int size = 1 + bl->lsb_reg_used; > > + > > +               ret = drm_dp_dpcd_read(aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, > > buf, size); > > +               if (ret != size) { > > +                       DRM_DEBUG_KMS("%s: Failed to read backlight level: > > %d\n", aux->name, ret); > > +                       return ret < 0 ? ret : -EIO; > > +               } > > + > > +               if (bl->lsb_reg_used) > > +                       return (buf[0] << 8) | buf[1]; > > +               else > > +                       return buf[0]; > > +       } > > + > > +       /* > > +        * If we're not in DPCD control mode yet, the programmed brightness > > value is meaningless and > > +        * the driver should assume max brightness > > +        */ > > +       return bl->max; > > +} > > + > > +/** > > + * drm_edp_backlight_init() - Probe a display panel's TCON using the > > standard VESA eDP backlight > > + * interface. > > + * @aux: The DP aux device to use for probing > > + * @bl: The &drm_edp_backlight_info struct to fill out with information on > > the backlight > > + * @driver_pwm_freq_hz: Optional PWM frequency from the driver in hz > > + * @edp_dpcd: A cached copy of the eDP DPCD > > + * @current_level: Where to store the probed brightness level > > + * @current_mode: Where to store the currently set backlight control mode > > + * > > + * Initializes a &drm_edp_backlight_info struct by probing @aux for it's > > backlight capabilities, > > + * along with also probing the current and maximum supported brightness > > levels. > > + * > > + * If @driver_pwm_freq_hz is non-zero, this will be used as the backlight > > frequency. Otherwise, the > > + * default frequency from the panel is used. > > + * > > + * Returns: %0 on success, negative error code on failure. > > + */ > > +int > > +drm_edp_backlight_init(struct drm_dp_aux *aux, struct > > drm_edp_backlight_info *bl, > > +                      u16 driver_pwm_freq_hz, const u8 > > edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE], > > +                      u16 *current_level, u8 *current_mode) > > +{ > > +       int ret; > > + > > +       if (edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) > > +               bl->aux_enable = true; > > +       if (edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT) > > +               bl->lsb_reg_used = true; > > + > > +       ret = drm_edp_backlight_probe_max(aux, bl, driver_pwm_freq_hz, > > edp_dpcd); > > +       if (ret < 0) > > +               return ret; > > + > > +       ret = drm_edp_backlight_probe_level(aux, bl, current_mode); > > +       if (ret < 0) > > +               return ret; > > +       *current_level = ret; > > + > > +       DRM_DEBUG_KMS("%s: Found backlight level=%d/%d > > pwm_freq_pre_divider=%d mode=%x\n", > > +                     aux->name, *current_level, bl->max, bl- > > >pwm_freq_pre_divider, *current_mode); > > +       DRM_DEBUG_KMS("%s: Backlight caps: pwmgen_bit_count=%d > > lsb_reg_used=%d aux_enable=%d\n", > > +                     aux->name, bl->pwmgen_bit_count, bl->lsb_reg_used, bl- > > >aux_enable); > > +       return 0; > > +} > > +EXPORT_SYMBOL(drm_edp_backlight_init); > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index 8450ab8fb245..2af208f1b467 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -263,8 +263,7 @@ struct intel_panel { > >                 /* DPCD backlight */ > >                 union { > >                         struct { > > -                               u8 pwmgen_bit_count; > > -                               u8 pwm_freq_pre_divider; > > +                               struct drm_edp_backlight_info info; > >                         } 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 62294967f430..1ae14bc9e3fe 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > @@ -268,109 +268,19 @@ 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) > > -{ > > -       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)) > > -               return; > > - > > -       if (drm_dp_dpcd_readb(&intel_dp->aux, > > DP_EDP_DISPLAY_CONTROL_REGISTER, > > -                             ®_val) < 0) { > > -               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); > > -       struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > -       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); > > -               return false; > > -       } > > - > > -       return (mode_reg & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) == > > -              DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD; > > -} > > - > > -/* > > - * 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_vesa_get_backlight(struct intel_connector > > *connector, enum pipe unused) > >  { > > -       struct intel_dp *intel_dp = intel_attached_dp(connector); > > -       struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > -       u8 read_val[2] = { 0x0 }; > > -       u16 level = 0; > > - > > -       /* > > -        * If we're not in DPCD control mode yet, the programmed brightness > > -        * value is meaningless and we should assume max brightness > > -        */ > > -       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)) < 0) { > > -               drm_dbg_kms(&i915->drm, "Failed to read DPCD register > > 0x%x\n", > > -                           DP_EDP_BACKLIGHT_BRIGHTNESS_MSB); > > -               return 0; > > -       } > > -       level = read_val[0]; > > -       if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT) > > -               level = (read_val[0] << 8 | read_val[1]); > > - > > -       return level; > > +       return connector->panel.backlight.level; > >  } > >   > > -/* > > - * Sends the current backlight level over the aux channel, checking if its > > using > > - * 8-bit or 16 bit value (MSB and LSB) > > - */ > >  static void > > -intel_dp_aux_vesa_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); > > -       struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > -       u8 vals[2] = { 0x0 }; > > - > > -       vals[0] = level; > > +       struct intel_panel *panel = &connector->panel; > > +       struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); > >   > > -       /* Write the MSB and/or LSB */ > > -       if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT) > > { > > -               vals[0] = (level & 0xFF00) >> 8; > > -               vals[1] = (level & 0xFF); > > -       } > > -       if (drm_dp_dpcd_write(&intel_dp->aux, > > DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, > > -                             vals, sizeof(vals)) < 0) { > > -               drm_dbg_kms(&i915->drm, > > -                           "Failed to write aux backlight level\n"); > > -               return; > > -       } > > +       drm_edp_backlight_set_level(&intel_dp->aux, &panel- > > >backlight.edp.vesa.info, level); > >  } > >   > >  static void > > @@ -378,176 +288,46 @@ intel_dp_aux_vesa_enable_backlight(const struct > > intel_crtc_state *crtc_state, > >                                    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); > > -       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 pwmgen_bit_count = panel->backlight.edp.vesa.pwmgen_bit_count; > > - > > -       if (drm_dp_dpcd_readb(&intel_dp->aux, > > -                       DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) > > { > > -               drm_dbg_kms(&i915->drm, "Failed to read DPCD register > > 0x%x\n", > > -                           DP_EDP_BACKLIGHT_MODE_SET_REGISTER); > > -               return; > > -       } > > - > > -       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: > > -               new_dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK; > > -               new_dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD; > > - > > -               if (drm_dp_dpcd_writeb(&intel_dp->aux, > > -                                      DP_EDP_PWMGEN_BIT_COUNT, > > -                                      pwmgen_bit_count) < 0) > > -                       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) { > > -               if (drm_dp_dpcd_writeb(&intel_dp->aux, > > DP_EDP_BACKLIGHT_FREQ_SET, > > -                                      panel- > > >backlight.edp.vesa.pwm_freq_pre_divider) == 1) > > -                       new_dpcd_buf |= > > DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; > > -               else > > -                       drm_dbg_kms(&i915->drm, "Failed to write aux > > backlight frequency\n"); > > -       } > > - > > -       if (new_dpcd_buf != dpcd_buf) { > > -               if (drm_dp_dpcd_writeb(&intel_dp->aux, > > -                       DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < > > 0) { > > -                       drm_dbg_kms(&i915->drm, > > -                                   "Failed to write aux backlight mode\n"); > > -               } > > -       } > > +       struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); > >   > > -       intel_dp_aux_vesa_set_backlight(conn_state, level); > > -       set_vesa_backlight_enable(intel_dp, true); > > +       drm_edp_backlight_enable(&intel_dp->aux, &panel- > > >backlight.edp.vesa.info, level); > >  } > >   > >  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_st > > ate->best_encoder)), > > -                                 false); > > -} > > - > > -/* > > - * Compute PWM frequency divider value based off the frequency provided to > > us by the vbt. > > - * The PWM Frequency is calculated as 27Mhz / (F x P). > > - * - Where F = PWM Frequency Pre-Divider value programmed by field 7:0 of > > the > > - *             EDP_BACKLIGHT_FREQ_SET register (DPCD Address 00728h) > > - * - 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 u32 intel_dp_aux_vesa_calc_max_backlight(struct intel_connector > > *connector) > > -{ > > -       struct drm_i915_private *i915 = to_i915(connector->base.dev); > > -       struct intel_dp *intel_dp = intel_attached_dp(connector); > > +       struct intel_connector *connector = > > to_intel_connector(old_conn_state->connector); > >         struct intel_panel *panel = &connector->panel; > > -       u32 max_backlight = 0; > > -       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; > > -       } > > - > > -       /* 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. > > -        */ > > -       freq = i915->vbt.backlight.pwm_freq_hz; > > -       drm_dbg_kms(&i915->drm, "VBT defined backlight frequency %u Hz\n", > > -                   freq); > > -       if (!freq) { > > -               drm_dbg_kms(&i915->drm, > > -                           "Use panel default backlight frequency\n"); > > -               return max_backlight; > > -       } > > - > > -       fxp = DIV_ROUND_CLOSEST(KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ), freq); > > - > > -       /* Use highest possible value of Pn for more granularity of > > brightness > > -        * adjustment while satifying the conditions below. > > -        * - Pn is in the range of Pn_min and Pn_max > > -        * - F is in the range of 1 and 255 > > -        * - FxP is within 25% of desired value. > > -        *   Note: 25% is arbitrary value and may need some tweak. > > -        */ > > -       if (drm_dp_dpcd_readb(&intel_dp->aux, > > -                             DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &pn_min) != > > 1) { > > -               drm_dbg_kms(&i915->drm, > > -                           "Failed to read pwmgen bit count cap min\n"); > > -               return max_backlight; > > -       } > > -       if (drm_dp_dpcd_readb(&intel_dp->aux, > > -                             DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, &pn_max) != > > 1) { > > -               drm_dbg_kms(&i915->drm, > > -                           "Failed to read pwmgen bit count cap max\n"); > > -               return max_backlight; > > -       } > > -       pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK; > > -       pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK; > > - > > -       /* Ensure frequency is within 25% of desired value */ > > -       fxp_min = DIV_ROUND_CLOSEST(fxp * 3, 4); > > -       fxp_max = DIV_ROUND_CLOSEST(fxp * 5, 4); > > - > > -       if (fxp_min < (1 << pn_min) || (255 << pn_max) < fxp_max) { > > -               drm_dbg_kms(&i915->drm, > > -                           "VBT defined backlight frequency out of > > range\n"); > > -               return max_backlight; > > -       } > > - > > -       for (pn = pn_max; pn >= pn_min; pn--) { > > -               f = clamp(DIV_ROUND_CLOSEST(fxp, 1 << pn), 1, 255); > > -               fxp_actual = f << pn; > > -               if (fxp_min <= fxp_actual && fxp_actual <= fxp_max) > > -                       break; > > -       } > > - > > -       drm_dbg_kms(&i915->drm, "Using eDP pwmgen bit count of %d\n", pn); > > -       if (drm_dp_dpcd_writeb(&intel_dp->aux, > > -                              DP_EDP_PWMGEN_BIT_COUNT, pn) < 0) { > > -               drm_dbg_kms(&i915->drm, > > -                           "Failed to write aux pwmgen bit count\n"); > > -               return max_backlight; > > -       } > > - > > -       panel->backlight.edp.vesa.pwmgen_bit_count = pn; > > -       if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP) > > -               panel->backlight.edp.vesa.pwm_freq_pre_divider = f; > > - > > -       max_backlight = (1 << pn) - 1; > > +       struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); > >   > > -       return max_backlight; > > +       drm_edp_backlight_disable(&intel_dp->aux, &panel- > > >backlight.edp.vesa.info); > >  } > >   > > -static int intel_dp_aux_vesa_setup_backlight(struct intel_connector > > *connector, > > -                                            enum pipe pipe) > > +static int intel_dp_aux_vesa_setup_backlight(struct intel_connector > > *connector, enum pipe pipe) > >  { > >         struct intel_panel *panel = &connector->panel; > > +       struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); > > +       struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > +       u16 current_level; > > +       u8 current_mode; > > +       int ret; > >   > > -       panel->backlight.max = > > intel_dp_aux_vesa_calc_max_backlight(connector); > > -       if (!panel->backlight.max) > > -               return -ENODEV; > > +       ret = drm_edp_backlight_init(&intel_dp->aux, &panel- > > >backlight.edp.vesa.info, > > +                                    i915->vbt.backlight.pwm_freq_hz, > > intel_dp->edp_dpcd, > > +                                    ¤t_level, ¤t_mode); > > +       if (ret < 0) > > +               return ret; > >   > > +       panel->backlight.max = panel->backlight.edp.vesa.info.max; > >         panel->backlight.min = 0; > > -       panel->backlight.level = intel_dp_aux_vesa_get_backlight(connector, > > pipe); > > -       panel->backlight.enabled = > > intel_dp_aux_vesa_backlight_dpcd_mode(connector) && > > -                                  panel->backlight.level != 0; > > +       if (current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) { > > +               panel->backlight.level = current_level; > > +               panel->backlight.enabled = panel->backlight.level != 0; > > +       } else { > > +               panel->backlight.level = panel->backlight.max; > > +               panel->backlight.enabled = false; > > +       } > >   > >         return 0; > >  } > > @@ -558,16 +338,12 @@ intel_dp_aux_supports_vesa_backlight(struct > > intel_connector *connector) > >         struct intel_dp *intel_dp = intel_attached_dp(connector); > >         struct drm_i915_private *i915 = dp_to_i915(intel_dp); > >   > > -       /* Check the eDP Display control capabilities registers to determine > > if > > -        * the panel can support backlight control over the aux channel. > > -        * > > -        * TODO: We currently only support AUX only backlight > > configurations, not backlights which > > +       /* TODO: We currently only support AUX only backlight > > configurations, not backlights which > >          * require a mix of PWM and AUX controls to work. In the mean time, > > these machines typically > >          * work just fine using normal PWM controls anyway. > >          */ > > -       if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP && > > -           (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > > -           (intel_dp->edp_dpcd[2] & > > DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { > > +       if ((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > > +           drm_edp_backlight_supported(intel_dp->edp_dpcd)) { > >                 drm_dbg_kms(&i915->drm, "AUX Backlight Control > > Supported!\n"); > >                 return true; > >         } > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > > index edffd1dcca3e..1eca0b42fc45 100644 > > --- a/include/drm/drm_dp_helper.h > > +++ b/include/drm/drm_dp_helper.h > > @@ -1790,6 +1790,24 @@ drm_dp_sink_can_do_video_without_timing_msa(const u8 > > dpcd[DP_RECEIVER_CAP_SIZE]) > >                 DP_MSA_TIMING_PAR_IGNORED; > >  } > >   > > +/** > > + * drm_edp_backlight_supported() - Check an eDP DPCD for VESA backlight > > support > > + * @edp_dpcd: The DPCD to check > > + * > > + * Note that currently this function will return %false for panels which > > support various DPCD > > + * backlight features but which require the brightness be set through PWM, > > and don't support setting > > + * the brightness level via the DPCD. This is a TODO. > > + * > > + * Returns: %True if @edp_dpcd indicates that VESA backlight controls are > > supported, %false > > + * otherwise > > + */ > > +static inline bool > > +drm_edp_backlight_supported(const u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE]) > > +{ > > +       return (edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && > > +               (edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP); > > +} > > + > >  /* > >   * DisplayPort AUX channel > >   */ > > @@ -2089,6 +2107,36 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum > > drm_dp_quirk quirk) > >         return desc->quirks & BIT(quirk); > >  } > >   > > +/** > > + * struct drm_edp_backlight_info - Probed eDP backlight info struct > > + * @pwmgen_bit_count: The pwmgen bit count > > + * @pwm_freq_pre_divider: The PWM frequency pre-divider value being used > > for this backlight, if any > > + * @max: The maximum backlight level that may be set > > + * @lsb_reg_used: Do we also write values to the > > DP_EDP_BACKLIGHT_BRIGHTNESS_LSB register? > > + * @aux_enable: Does the panel support the AUX enable cap? > > + * > > + * This structure contains various data about an eDP backlight, which can > > be populated by using > > + * drm_edp_backlight_init(). > > + */ > > +struct drm_edp_backlight_info { > > +       u8 pwmgen_bit_count; > > +       u8 pwm_freq_pre_divider; > > +       u16 max; > > + > > +       bool lsb_reg_used : 1; > > +       bool aux_enable : 1; > > +}; > > + > > +int > > +drm_edp_backlight_init(struct drm_dp_aux *aux, struct > > drm_edp_backlight_info *bl, > > +                      u16 driver_pwm_freq_hz, const u8 > > edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE], > > +                      u16 *current_level, u8 *current_mode); > > +int drm_edp_backlight_set_level(struct drm_dp_aux *aux, const struct > > drm_edp_backlight_info *bl, > > +                               u16 level); > > +int drm_edp_backlight_enable(struct drm_dp_aux *aux, const struct > > drm_edp_backlight_info *bl, > > +                            u16 level); > > +int drm_edp_backlight_disable(struct drm_dp_aux *aux, const struct > > drm_edp_backlight_info *bl); > > + > >  #ifdef CONFIG_DRM_DP_CEC > >  void drm_dp_cec_irq(struct drm_dp_aux *aux); > >  void drm_dp_cec_register_connector(struct drm_dp_aux *aux, > > -- > > 2.29.2 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've asked me a question, are waiting for a review/merge on a patch, etc. and I haven't responded in a while, please feel free to send me another email to check on my status. I don't bite!