| Message ID | 20221110183853.3678209-3-jagan@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm: bridge: Add Samsung MIPI DSIM bridge | expand |
On 11/10/22 19:38, Jagan Teki wrote: > HSA/HBP/HFP/HSE mode bits in Exynos DSI host specify a naming > conversion as 'disable mode bit' due to its bit definition, > 0 = Enable and 1 = Disable. > > Fix the naming convention of the mode bits. > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > index b5305b145ddb..fce7f0a7e4ee 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > @@ -75,10 +75,10 @@ > #define DSIM_MAIN_PIX_FORMAT_RGB565 (0x4 << 12) > #define DSIM_SUB_VC (((x) & 0x3) << 16) > #define DSIM_MAIN_VC (((x) & 0x3) << 18) > -#define DSIM_HSA_MODE (1 << 20) > -#define DSIM_HBP_MODE (1 << 21) > -#define DSIM_HFP_MODE (1 << 22) > -#define DSIM_HSE_MODE (1 << 23) > +#define DSIM_HSA_DISABLE (1 << 20) > +#define DSIM_HBP_DISABLE (1 << 21) > +#define DSIM_HFP_DISABLE (1 << 22) > +#define DSIM_HSE_DISABLE (1 << 23) Those four bits are called Hxx_DISABLE_MODE in the MX8M{M,N,P}RM at least, so keep both suffixes . A separate patch which turns those bits to BIT() macro would be nice.
On Sun, Nov 13, 2022 at 5:55 AM Marek Vasut <marex@denx.de> wrote: > > On 11/10/22 19:38, Jagan Teki wrote: > > HSA/HBP/HFP/HSE mode bits in Exynos DSI host specify a naming > > conversion as 'disable mode bit' due to its bit definition, > > 0 = Enable and 1 = Disable. > > > > Fix the naming convention of the mode bits. > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > index b5305b145ddb..fce7f0a7e4ee 100644 > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > @@ -75,10 +75,10 @@ > > #define DSIM_MAIN_PIX_FORMAT_RGB565 (0x4 << 12) > > #define DSIM_SUB_VC (((x) & 0x3) << 16) > > #define DSIM_MAIN_VC (((x) & 0x3) << 18) > > -#define DSIM_HSA_MODE (1 << 20) > > -#define DSIM_HBP_MODE (1 << 21) > > -#define DSIM_HFP_MODE (1 << 22) > > -#define DSIM_HSE_MODE (1 << 23) > > +#define DSIM_HSA_DISABLE (1 << 20) > > +#define DSIM_HBP_DISABLE (1 << 21) > > +#define DSIM_HFP_DISABLE (1 << 22) > > +#define DSIM_HSE_DISABLE (1 << 23) > > Those four bits are called Hxx_DISABLE_MODE in the MX8M{M,N,P}RM at > least, so keep both suffixes . Okay, I will update the suffixes. > > A separate patch which turns those bits to BIT() macro would be nice. I plan to do this conversion for all bits once this basic patchset merges. I hope it is okay with it. Jagan.
On 10.11.22 19:38, Jagan Teki wrote: > HSA/HBP/HFP/HSE mode bits in Exynos DSI host specify a naming > conversion as 'disable mode bit' due to its bit definition, > 0 = Enable and 1 = Disable. > > Fix the naming convention of the mode bits. > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > index b5305b145ddb..fce7f0a7e4ee 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > @@ -75,10 +75,10 @@ > #define DSIM_MAIN_PIX_FORMAT_RGB565 (0x4 << 12) > #define DSIM_SUB_VC (((x) & 0x3) << 16) > #define DSIM_MAIN_VC (((x) & 0x3) << 18) > -#define DSIM_HSA_MODE (1 << 20) > -#define DSIM_HBP_MODE (1 << 21) > -#define DSIM_HFP_MODE (1 << 22) > -#define DSIM_HSE_MODE (1 << 23) > +#define DSIM_HSA_DISABLE (1 << 20) > +#define DSIM_HBP_DISABLE (1 << 21) > +#define DSIM_HFP_DISABLE (1 << 22) > +#define DSIM_HSE_DISABLE (1 << 23) > #define DSIM_AUTO_MODE (1 << 24) > #define DSIM_VIDEO_MODE (1 << 25) > #define DSIM_BURST_MODE (1 << 26) > @@ -804,13 +804,13 @@ static int exynos_dsi_init_link(struct exynos_dsi *dsi) > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_AUTO_VERT) > reg |= DSIM_AUTO_MODE; > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_HSE) > - reg |= DSIM_HSE_MODE; > + reg |= DSIM_HSE_DISABLE; Please add a comment to explain that the DSIM_HSE_DISABLE bit as named in the datasheet actually has inverted logic (set = HSE enabled). > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP) > - reg |= DSIM_HFP_MODE; > + reg |= DSIM_HFP_DISABLE; > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP) > - reg |= DSIM_HBP_MODE; > + reg |= DSIM_HBP_DISABLE; > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA) > - reg |= DSIM_HSA_MODE; > + reg |= DSIM_HSA_DISABLE; > } > > if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)
On Mon, Dec 5, 2022 at 5:29 PM Frieder Schrempf <frieder.schrempf@kontron.de> wrote: > > On 10.11.22 19:38, Jagan Teki wrote: > > HSA/HBP/HFP/HSE mode bits in Exynos DSI host specify a naming > > conversion as 'disable mode bit' due to its bit definition, > > 0 = Enable and 1 = Disable. > > > > Fix the naming convention of the mode bits. > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > index b5305b145ddb..fce7f0a7e4ee 100644 > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > @@ -75,10 +75,10 @@ > > #define DSIM_MAIN_PIX_FORMAT_RGB565 (0x4 << 12) > > #define DSIM_SUB_VC (((x) & 0x3) << 16) > > #define DSIM_MAIN_VC (((x) & 0x3) << 18) > > -#define DSIM_HSA_MODE (1 << 20) > > -#define DSIM_HBP_MODE (1 << 21) > > -#define DSIM_HFP_MODE (1 << 22) > > -#define DSIM_HSE_MODE (1 << 23) > > +#define DSIM_HSA_DISABLE (1 << 20) > > +#define DSIM_HBP_DISABLE (1 << 21) > > +#define DSIM_HFP_DISABLE (1 << 22) > > +#define DSIM_HSE_DISABLE (1 << 23) > > #define DSIM_AUTO_MODE (1 << 24) > > #define DSIM_VIDEO_MODE (1 << 25) > > #define DSIM_BURST_MODE (1 << 26) > > @@ -804,13 +804,13 @@ static int exynos_dsi_init_link(struct exynos_dsi *dsi) > > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_AUTO_VERT) > > reg |= DSIM_AUTO_MODE; > > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_HSE) > > - reg |= DSIM_HSE_MODE; > > + reg |= DSIM_HSE_DISABLE; > > Please add a comment to explain that the DSIM_HSE_DISABLE bit as named > in the datasheet actually has inverted logic (set = HSE enabled). Yes, I have the V9 series with all relevant fixes and updates - waiting for the responses on the host init patch 06/14 Jagan.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index b5305b145ddb..fce7f0a7e4ee 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -75,10 +75,10 @@ #define DSIM_MAIN_PIX_FORMAT_RGB565 (0x4 << 12) #define DSIM_SUB_VC (((x) & 0x3) << 16) #define DSIM_MAIN_VC (((x) & 0x3) << 18) -#define DSIM_HSA_MODE (1 << 20) -#define DSIM_HBP_MODE (1 << 21) -#define DSIM_HFP_MODE (1 << 22) -#define DSIM_HSE_MODE (1 << 23) +#define DSIM_HSA_DISABLE (1 << 20) +#define DSIM_HBP_DISABLE (1 << 21) +#define DSIM_HFP_DISABLE (1 << 22) +#define DSIM_HSE_DISABLE (1 << 23) #define DSIM_AUTO_MODE (1 << 24) #define DSIM_VIDEO_MODE (1 << 25) #define DSIM_BURST_MODE (1 << 26) @@ -804,13 +804,13 @@ static int exynos_dsi_init_link(struct exynos_dsi *dsi) if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_AUTO_VERT) reg |= DSIM_AUTO_MODE; if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_HSE) - reg |= DSIM_HSE_MODE; + reg |= DSIM_HSE_DISABLE; if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP) - reg |= DSIM_HFP_MODE; + reg |= DSIM_HFP_DISABLE; if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP) - reg |= DSIM_HBP_MODE; + reg |= DSIM_HBP_DISABLE; if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA) - reg |= DSIM_HSA_MODE; + reg |= DSIM_HSA_DISABLE; } if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)
HSA/HBP/HFP/HSE mode bits in Exynos DSI host specify a naming conversion as 'disable mode bit' due to its bit definition, 0 = Enable and 1 = Disable. Fix the naming convention of the mode bits. Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)