[PATCH] drm/msm/hdmi: mark interlace_allowed as true

[Dmitry Baryshkov]

On Fri, Nov 01, 2024 at 05:40:46PM -0700, Abhinav Kumar wrote:
> 
> 
> On 11/1/2024 3:26 PM, Dmitry Baryshkov wrote:
> > On Fri, 1 Nov 2024 at 23:41, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
> > > 
> > > 
> > > 
> > > On 10/18/2024 2:10 PM, Dmitry Baryshkov wrote:
> > > > The MSM HDMI driver supports interlaced modes. Set the corresponding
> > > > flag to allow interlaced modes on the corresponding connectors.
> > > > 
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> > > > ---
> > > >    drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 1 +
> > > >    1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > > > index 4a5b5112227f..643c152e6380 100644
> > > > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > > > @@ -336,6 +336,7 @@ int msm_hdmi_bridge_init(struct hdmi *hdmi)
> > > >        bridge->funcs = &msm_hdmi_bridge_funcs;
> > > >        bridge->ddc = hdmi->i2c;
> > > >        bridge->type = DRM_MODE_CONNECTOR_HDMIA;
> > > > +     bridge->interlace_allowed = true;
> > > >        bridge->ops = DRM_BRIDGE_OP_HPD |
> > > >                DRM_BRIDGE_OP_DETECT |
> > > >                DRM_BRIDGE_OP_EDID;
> > > > 
> > > 
> > > I had quite a bit of discussion on this internally because this spans
> > > quite a few generations of chipsets.
> > > 
> > > On very old hardware, even before msm8996, there was dedicated hardware
> > > de-interlacer. But even on msm8996 or other HDMI supported chipsets
> > > where the handling of if (mode->flags & DRM_MODE_FLAG_INTERLACE) is
> > > present, these were because its carry forward of older interface code.
> > > 
> > > The way we handle interlaced formats today, is software needs to handle
> > > the part of dividing height / 2 and width * 2 and adjust the source crop
> > > if necessary. This part has moved to userspace for recent chips.
> > > 
> > > Othwerise, we will need to add this part in the dpu driver to adjust
> > > this. I am not seeing this part there yet. So may I know how you
> > > validated this change? Something similar to :
> > > 
> > > https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/LE.UM.1.3.r3.25/drivers/gpu/drm/msm/sde/sde_plane.c#L1340
> > > 
> > > If we add this part first to dpu code, then we can mark interlace_allowed.
> > 
> > I think you are mixing the interlaced formats and interlaced output.
> > The code that you have pointed to is related to hardware deinterlacing
> > - in other words taking the interlaced framebuffer and outputting it
> > to the progressive display.
> > 
> > The interlace_allowed flag controls a different feature - filtering of
> > the internalced modes (aka 576i, 1080i, etc). In this case we are
> > using progressive frames, but the HDMI outputs a picture as two
> > separate fields. I have validated this by outputting image (modetest)
> > to the external HDMI display on IFC6410 and on DB820c boards.
> > 
> 
> Yes I did think that this was to show interlaced content but that being
> said, I traced through the HDMI code a bit, it does have support for
> changing the HDMI timing but without the support of dpu, progressive content
> really cannot be converted to interlaced. So I think the HDMI pieces there
> were supposed to go along with the rest of the dpu pipeline that is the
> entire pipeline shows out interlaced content. But dpu support for giving out
> interlaced content is not there, so this hdmi piece by itself is not
> complete enough to mark interlace_allowed as true.

I could not find corresponding bits in the original fbdev or SDE
drivers. My quick tests showed the correct context, but most likely I
need to revertify that. Unfortunately next week I won't be able to run
the tests, so this gets into the 6.14 area.

-- 
With best wishes
Dmitry