Hi,
On 11/2/23 16:12, Andy Shevchenko wrote:
> It's a dirty hack in the driver that pokes GPIO registers behind
> the driver's back. Moreoever it might be problematic as simultaneous
> I/O may hang the system, see the commit 0bd50d719b00 ("pinctrl:
> cherryview: prevent concurrent access to GPIO controllers") for
> the details. Taking all this into consideration replace the hack
> with proper GPIO APIs being used.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 47 +++++---------------
> 1 file changed, 10 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index b1736c1301ea..ffc65c943b11 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -66,19 +66,6 @@ struct i2c_adapter_lookup {
> #define CHV_GPIO_IDX_START_SW 100
> #define CHV_GPIO_IDX_START_SE 198
>
> -#define CHV_VBT_MAX_PINS_PER_FMLY 15
> -
> -#define CHV_GPIO_PAD_CFG0(f, i) (0x4400 + (f) * 0x400 + (i) * 8)
> -#define CHV_GPIO_GPIOEN (1 << 15)
> -#define CHV_GPIO_GPIOCFG_GPIO (0 << 8)
> -#define CHV_GPIO_GPIOCFG_GPO (1 << 8)
> -#define CHV_GPIO_GPIOCFG_GPI (2 << 8)
> -#define CHV_GPIO_GPIOCFG_HIZ (3 << 8)
> -#define CHV_GPIO_GPIOTXSTATE(state) ((!!(state)) << 1)
> -
> -#define CHV_GPIO_PAD_CFG1(f, i) (0x4400 + (f) * 0x400 + (i) * 8 + 4)
> -#define CHV_GPIO_CFGLOCK (1 << 31)
> -
> /* ICL DSI Display GPIO Pins */
> #define ICL_GPIO_DDSP_HPD_A 0
> #define ICL_GPIO_L_VDDEN_1 1
> @@ -278,23 +265,21 @@ static void chv_gpio_set_value(struct intel_connector *connector,
> u8 gpio_source, u8 gpio_index, bool value)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> - u16 cfg0, cfg1;
> - u16 family_num;
> - u8 port;
>
> if (connector->panel.vbt.dsi.seq_version >= 3) {
> if (gpio_index >= CHV_GPIO_IDX_START_SE) {
> /* XXX: it's unclear whether 255->57 is part of SE. */
> - gpio_index -= CHV_GPIO_IDX_START_SE;
> - port = CHV_IOSF_PORT_GPIO_SE;
> + soc_exec_opaque_gpio(connector, gpio_index, "INT33FF:03", "Panel SE",
> + gpio_index - CHV_GPIO_IDX_START_SW, value);
The "gpio_index - CHV_GPIO_IDX_START_SW" here needs to be "gpio_index - CHV_GPIO_IDX_START_SE".
Also this patch needs s/soc_exec_opaque_gpio/soc_opaque_gpio_set_value/ to compile ...
Regards,
Hans
> } else if (gpio_index >= CHV_GPIO_IDX_START_SW) {
> - gpio_index -= CHV_GPIO_IDX_START_SW;
> - port = CHV_IOSF_PORT_GPIO_SW;
> + soc_exec_opaque_gpio(connector, gpio_index, "INT33FF:00", "Panel SW",
> + gpio_index - CHV_GPIO_IDX_START_SW, value);
> } else if (gpio_index >= CHV_GPIO_IDX_START_E) {
> - gpio_index -= CHV_GPIO_IDX_START_E;
> - port = CHV_IOSF_PORT_GPIO_E;
> + soc_exec_opaque_gpio(connector, gpio_index, "INT33FF:02", "Panel E",
> + gpio_index - CHV_GPIO_IDX_START_E, value);
> } else {
> - port = CHV_IOSF_PORT_GPIO_N;
> + soc_exec_opaque_gpio(connector, gpio_index, "INT33FF:01", "Panel N",
> + gpio_index - CHV_GPIO_IDX_START_N, value);
> }
> } else {
> /* XXX: The spec is unclear about CHV GPIO on seq v2 */
> @@ -311,21 +296,9 @@ static void chv_gpio_set_value(struct intel_connector *connector,
> return;
> }
>
> - port = CHV_IOSF_PORT_GPIO_N;
> + soc_exec_opaque_gpio(connector, gpio_index, "INT33FF:01", "Panel N",
> + gpio_index - CHV_GPIO_IDX_START_N, value);
> }
> -
> - family_num = gpio_index / CHV_VBT_MAX_PINS_PER_FMLY;
> - gpio_index = gpio_index % CHV_VBT_MAX_PINS_PER_FMLY;
> -
> - cfg0 = CHV_GPIO_PAD_CFG0(family_num, gpio_index);
> - cfg1 = CHV_GPIO_PAD_CFG1(family_num, gpio_index);
> -
> - vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_GPIO));
> - vlv_iosf_sb_write(dev_priv, port, cfg1, 0);
> - vlv_iosf_sb_write(dev_priv, port, cfg0,
> - CHV_GPIO_GPIOEN | CHV_GPIO_GPIOCFG_GPO |
> - CHV_GPIO_GPIOTXSTATE(value));
> - vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_GPIO));
> }
>
> static void bxt_gpio_set_value(struct intel_connector *connector,
