On Mon, Oct 28, 2024 at 02:52:09PM +0100, Maxime Ripard wrote:
> On Mon, Oct 28, 2024 at 01:36:58PM +0100, Marek Vasut wrote:
> > On 10/28/24 10:25 AM, Maxime Ripard wrote:
> > > On Sat, Oct 26, 2024 at 06:10:01AM +0200, Marek Vasut wrote:
> > > > Horizontal Timing Control0 Register 1/2 (HTIM01/HTIM02) Register
> > > > bitfields description state "These bits must be multiple of even
> > > > pixel". It is not possible to simply align every bitfield to the
> > > > nearest even pixel, because that would unalign the line width and
> > > > cause visible distortion. Instead, attempt to re-align the timings
> > > > such that the hardware requirement is fulfilled without changing
> > > > the line width if at all possible.
> > > > 
> > > > Warn the user in case a panel with odd active pixel width or full
> > > > line width is used, this is not possible to support with this one
> > > > bridge.
> > > > 
> > > > Signed-off-by: Marek Vasut <[email protected]>
> > > > ---
> > > > Cc: Andrzej Hajda <[email protected]>
> > > > Cc: David Airlie <[email protected]>
> > > > Cc: Jernej Skrabec <[email protected]>
> > > > Cc: Jonas Karlman <[email protected]>
> > > > Cc: Laurent Pinchart <[email protected]>
> > > > Cc: Maarten Lankhorst <[email protected]>
> > > > Cc: Maxime Ripard <[email protected]>
> > > > Cc: Neil Armstrong <[email protected]>
> > > > Cc: Robert Foss <[email protected]>
> > > > Cc: Simona Vetter <[email protected]>
> > > > Cc: Thomas Zimmermann <[email protected]>
> > > > Cc: [email protected]
> > > > ---
> > > >   drivers/gpu/drm/bridge/tc358767.c | 63 +++++++++++++++++++++++++++++--
> > > >   1 file changed, 60 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/bridge/tc358767.c 
> > > > b/drivers/gpu/drm/bridge/tc358767.c
> > > > index 0a6894498267e..7968183510e63 100644
> > > > --- a/drivers/gpu/drm/bridge/tc358767.c
> > > > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > > > @@ -901,6 +901,63 @@ static int tc_set_common_video_mode(struct tc_data 
> > > > *tc,
> > > >         int vsync_len = mode->vsync_end - mode->vsync_start;
> > > >         int ret;
> > > > +       /*
> > > > +        * Horizontal Timing Control0 Register 1/2 (HTIM01/HTIM02) 
> > > > Register
> > > > +        * bitfields description state "These bits must be multiple of 
> > > > even
> > > > +        * pixel". It is not possible to simply align every bitfield to 
> > > > the
> > > > +        * nearest even pixel, because that would unalign the line 
> > > > width.
> > > > +        * Instead, attempt to re-align the timings.
> > > > +        */
> > > > +
> > > > +       /* Panels with odd active pixel count are not supported by the 
> > > > bridge */
> > > > +       if (mode->hdisplay & 1)
> > > > +               dev_warn(tc->dev, "Panels with odd pixel count per 
> > > > active line are not supported.\n");
> > > > +
> > > > +       /* HPW is odd */
> > > > +       if (hsync_len & 1) {
> > > > +               /* Make sure there is some margin left */
> > > > +               if (left_margin >= 2) {
> > > > +                       /* Align HPW up */
> > > > +                       hsync_len++;
> > > > +                       left_margin--;
> > > > +               } else if (right_margin >= 2) {
> > > > +                       /* Align HPW up */
> > > > +                       hsync_len++;
> > > > +                       right_margin--;
> > > > +               } else if (hsync_len > 2) {
> > > > +                       /* Align HPW down as last-resort option */
> > > > +                       hsync_len--;
> > > > +                       left_margin++;
> > > > +               } else {
> > > > +                       dev_warn(tc->dev, "HPW is odd, not enough 
> > > > margins to compensate.\n");
> > > > +               }
> > > > +       }
> > > > +
> > > > +       /* HBP is odd (HPW is surely even now) */
> > > > +       if (left_margin & 1) {
> > > > +               /* Make sure there is some margin left */
> > > > +               if (right_margin >= 2) {
> > > > +                       /* Align HBP up */
> > > > +                       left_margin++;
> > > > +                       right_margin--;
> > > > +               } else if (hsync_len > 2) {
> > > > +                       /* HPW is surely even and > 2, which means at 
> > > > least 4 */
> > > > +                       hsync_len -= 2;
> > > > +                       /*
> > > > +                        * Subtract 2 from sync pulse and distribute it 
> > > > between
> > > > +                        * margins. This aligns HBP and keeps HPW 
> > > > aligned.
> > > > +                        */
> > > > +                       left_margin++;
> > > > +                       right_margin++;
> > > > +               } else {
> > > > +                       dev_warn(tc->dev, "HBP is odd, not enough 
> > > > pixels to compensate.\n");
> > > > +               }
> > > > +       }
> > > > +
> > > > +       /* HFP is odd (HBP and HPW is surely even now) */
> > > > +       if (right_margin & 1)
> > > > +               dev_warn(tc->dev, "HFP is odd, panels with odd pixel 
> > > > count per full line are not supported.\n");
> > > > +
> > > 
> > > This should all happen in atomic_check, and reject modes that can't
> > > be supported.
> 
> > No, that would reject panels I need to support and which can be
> > supported by this bridge.
> 
> Then drop the warnings, either you support it or you don't.

Oh, and update the commit log, because so far it claims that you can't
support those panels.

Maxime

Attachment: signature.asc
Description: PGP signature

Reply via email to