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@xxxxxxxxxxx> 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@xxxxxxxxxx> > > > > --- > > > > 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