Messages in this thread Patch in this message | | | Date | Tue, 15 Aug 2023 19:21:37 +0200 | From | Maxim Schwalm <> | Subject | Re: [PATCH 02/11] drm/bridge: tc358768: Fix bit updates |
| |
On 14.08.23 08:34, Tomi Valkeinen wrote: > On 13/08/2023 03:23, Maxim Schwalm wrote: >> Hi, >> >> On 11.08.23 19:02, Tomi Valkeinen wrote: >>> On 11/08/2023 19:23, Péter Ujfalusi wrote: >>>> >>>> >>>> On 04/08/2023 13:44, Tomi Valkeinen wrote: >>>>> The driver has a few places where it does: >>>>> >>>>> if (thing_is_enabled_in_config) >>>>> update_thing_bit_in_hw() >>>>> >>>>> This means that if the thing is _not_ enabled, the bit never gets >>>>> cleared. This affects the h/vsyncs and continuous DSI clock bits. >>>> >>>> I guess the idea was to keep the reset value unless it needs to be flipped. >>>> >>>>> >>>>> Fix the driver to always update the bit. >>>>> >>>>> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver") >>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> >>>>> --- >>>>> drivers/gpu/drm/bridge/tc358768.c | 13 +++++++------ >>>>> 1 file changed, 7 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c >>>>> index bc97a837955b..b668f77673c3 100644 >>>>> --- a/drivers/gpu/drm/bridge/tc358768.c >>>>> +++ b/drivers/gpu/drm/bridge/tc358768.c >>>>> @@ -794,8 +794,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) >>>>> val |= BIT(i + 1); >>>>> tc358768_write(priv, TC358768_HSTXVREGEN, val); >>>>> >>>>> - if (!(mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)) >>>>> - tc358768_write(priv, TC358768_TXOPTIONCNTRL, 0x1); >>>>> + tc358768_write(priv, TC358768_TXOPTIONCNTRL, >>>>> + (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) ? 0 : BIT(0)); >>>>> >>>>> /* TXTAGOCNT[26:16] RXTASURECNT[10:0] */ >>>>> val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4); >>>>> @@ -861,11 +861,12 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) >>>>> tc358768_write(priv, TC358768_DSI_HACT, hact); >>>>> >>>>> /* VSYNC polarity */ >>>>> - if (!(mode->flags & DRM_MODE_FLAG_NVSYNC)) >>>>> - tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5), BIT(5)); >>>>> + tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5), >>>>> + (mode->flags & DRM_MODE_FLAG_PVSYNC) ? BIT(5) : 0); >>>> >>>> Was this the reverse before and should be: >>>> (mode->flags & DRM_MODE_FLAG_PVSYNC) ? 0 : BIT(5) >>> >>> Bit 5 is 1 for active high vsync polarity. The test was previously >>> !nvsync, i.e. the same as pvsync. >> >> this statement doesn't seem to be true, since this change causes a >> regression on the Asus TF700T. Apparently, !nvsync is true and pvsync is >> false in the present case. > > panasonic_vvx10f004b00_mode in panel_simple.c doesn't seem to have mode > flags set. I would say that means the panel doesn't care about the sync > polarities (which obviously is not the case), but maybe there's an > assumption that if sync polarities are not set, the default is... > positive? But I can't find any mention about this. > > Does it work for you if you set the polarities in > panasonic_vvx10f004b00_mode?
The panel seems to work with either negative or positive H-/Vsync in conjunction with the attached patch from Thierry. Currently, the display controller is unconditionally programmed for positive H-/Vsync though. What should be done in this case?
BTW, the vendor kernel configures the display controller as well as the bridge for negative H-/Vsync.
Best regards, MaximFrom efc4f9108b3f44d3e00146bdb97c906f15e2a068 Mon Sep 17 00:00:00 2001 From: Thierry Reding <treding@nvidia.com> Date: Mon, 3 Aug 2015 13:34:36 +0200 Subject: [PATCH] drm/tegra: rgb: Parameterize V- and H-sync polarities
The polarities of the V- and H-sync signals are encoded as flags in the display mode, so use the existing information to setup the signals for the RGB interface.
Signed-off-by: Thierry Reding <treding@nvidia.com> --- drivers/gpu/drm/tegra/rgb.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c index 79566c9ea8ff..cce422c2d6a6 100644 --- a/drivers/gpu/drm/tegra/rgb.c +++ b/drivers/gpu/drm/tegra/rgb.c @@ -99,6 +99,7 @@ static void tegra_rgb_encoder_disable(struct drm_encoder *encoder) static void tegra_rgb_encoder_enable(struct drm_encoder *encoder) { + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; struct tegra_output *output = encoder_to_output(encoder); struct tegra_rgb *rgb = to_rgb(output); u32 value; @@ -108,10 +109,21 @@ static void tegra_rgb_encoder_enable(struct drm_encoder *encoder) value = DE_SELECT_ACTIVE | DE_CONTROL_NORMAL; tegra_dc_writel(rgb->dc, value, DC_DISP_DATA_ENABLE_OPTIONS); - /* XXX: parameterize? */ + /* configure H- and V-sync signal polarities */ value = tegra_dc_readl(rgb->dc, DC_COM_PIN_OUTPUT_POLARITY(1)); - value &= ~LVS_OUTPUT_POLARITY_LOW; - value &= ~LHS_OUTPUT_POLARITY_LOW; + + if (mode->flags & DRM_MODE_FLAG_PHSYNC) + value &= ~LHS_OUTPUT_POLARITY_LOW; + + if (mode->flags & DRM_MODE_FLAG_NHSYNC) + value |= LHS_OUTPUT_POLARITY_LOW; + + if (mode->flags & DRM_MODE_FLAG_PVSYNC) + value &= ~LVS_OUTPUT_POLARITY_LOW; + + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + value |= LVS_OUTPUT_POLARITY_LOW; + tegra_dc_writel(rgb->dc, value, DC_COM_PIN_OUTPUT_POLARITY(1)); /* XXX: parameterize? */ -- 2.39.2
| |