On Tue, 30 Oct 2018, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Tue, Oct 30, 2018 at 01:56:40PM +0200, Jani Nikula wrote:
>> From: Madhav Chauhan <madhav.chauhan@xxxxxxxxx>
>>
>> For ICELAKE DSI, Display Pins are the only GPIOs
>> that need to be programmed. So DSI driver should have
>> its own implementation to toggle these pins based on
>> GPIO info coming from VBT sequences instead of using
>> platform specific GPIO driver.
>>
>> Signed-off-by: Madhav Chauhan <madhav.chauhan@xxxxxxxxx>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/intel_dsi_vbt.c | 46 +++++++++++++++++++++++++++++++++++-
>> 1 file changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> index 8177305b9c87..04423248bbd7 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> @@ -334,6 +334,48 @@ static void bxt_exec_gpio(struct drm_i915_private *dev_priv,
>> gpiod_set_value(gpio_desc, value);
>> }
>>
>> +static void icl_exec_gpio(struct drm_i915_private *dev_priv,
>> + u8 gpio_source, u8 gpio_index, bool value)
>> +{
>> + u32 val;
>> +
>> + switch (gpio_index) {
>> + case ICL_GPIO_DDSP_HPD_A:
>> + val = I915_READ(SHOTPLUG_CTL_DDI);
>> + val &= ~ICP_DDIA_HPD_ENABLE;
>> + I915_WRITE(SHOTPLUG_CTL_DDI, val);
>> + val = I915_READ(SHOTPLUG_CTL_DDI);
>> + if (value)
>> + val |= ICP_DDIA_HPD_OP_DRIVE_1;
>> + else
>> + val &= ~ICP_DDIA_HPD_OP_DRIVE_1;
>> +
>> + I915_WRITE(SHOTPLUG_CTL_DDI, val);
>
> How badly is this thing going to race with the hotplug irq handler?
>
>> + break;
>> + case ICL_GPIO_L_VDDEN_1:
>> + val = I915_READ(ICP_PP_CONTROL(1));
>> + if (value)
>> + val |= PWR_STATE_TARGET;
>> + else
>> + val &= ~PWR_STATE_TARGET;
>> + I915_WRITE(ICP_PP_CONTROL(1), val);
>> + break;
>> + case ICL_GPIO_L_BKLTEN_1:
>> + val = I915_READ(ICP_PP_CONTROL(1));
>> + if (value)
>> + val |= BACKLIGHT_ENABLE;
>> + else
>> + val &= ~BACKLIGHT_ENABLE;
>> + I915_WRITE(ICP_PP_CONTROL(1), val);
>
> :( What a horror show. So basically we're trying to pretend the power
> sequencer state machine doesn't even exist. Is there some bit somewhere
> we can actually use to disable the state machine? If not I think this
> thing needs much more care.
Frankly I didn't look at the patches towards the end of the series all
that much. Just included them all for completeness.
Agreed, looks pretty bad. :(
BR,
Jani.
>
>> + break;
>> + default:
>> + /* TODO: Add support for remaining GPIOs */
>> + DRM_ERROR("Invalid GPIO no from VBT\n");
>> + break;
>> + }
>> +}
>> +
>> static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>> {
>> struct drm_device *dev = intel_dsi->base.base.dev;
>> @@ -357,7 +399,9 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>> /* pull up/down */
>> value = *data++ & 1;
>>
>> - if (IS_VALLEYVIEW(dev_priv))
>> + if (IS_ICELAKE(dev_priv))
>> + icl_exec_gpio(dev_priv, gpio_source, gpio_index, value);
>> + else if (IS_VALLEYVIEW(dev_priv))
>> vlv_exec_gpio(dev_priv, gpio_source, gpio_number, value);
>> else if (IS_CHERRYVIEW(dev_priv))
>> chv_exec_gpio(dev_priv, gpio_source, gpio_number, value);
>> --
>> 2.11.0
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
