mgag200: Only set VIDRST bits in CRTC modesetting

Jocelyn Falempe Thu, 04 Jul 2024 05:04:05 -0700



On 03/07/2024 15:40, Thomas Zimmermann wrote:

The VRSTEN and HRSTEN bits control whether a CRTC synchronizes its
display signal with an external source on the VIDRST pin. The G200WB
and G200EW3 models synchronize with a BMC chip, but different external
video encoders, such as the Matrox Maven, can also be attached to the
pin.


If I understand correctly, it's a kind of VSYNC with the BMC, to avoid
tearing when using the remote console ?


Only set VRSTEN and HRSTEN bits in the CRTC mode-setting code, so the
driver maintains the bits independently from the BMC. Add the field
set_vidrst to the CRTC state for this purpose. Off by default, control
the CRTC VIDRST setting from the BMC's atomic_check helper. So if a
BMC (or another external clock) requires synchronization, it instructs
the CRTC to synchronize. >
Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
---
  drivers/gpu/drm/mgag200/mgag200_bmc.c    | 26 +++++++++++++++++++-----
  drivers/gpu/drm/mgag200/mgag200_drv.h    |  5 ++++-
  drivers/gpu/drm/mgag200/mgag200_g200er.c |  2 +-
  drivers/gpu/drm/mgag200/mgag200_g200ev.c |  2 +-
  drivers/gpu/drm/mgag200/mgag200_g200se.c |  2 +-
  drivers/gpu/drm/mgag200/mgag200_mode.c   | 11 ++++++----
  6 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_bmc.c 
b/drivers/gpu/drm/mgag200/mgag200_bmc.c
index 23ef85aa7e37..cb5400333862 100644
--- a/drivers/gpu/drm/mgag200/mgag200_bmc.c
+++ b/drivers/gpu/drm/mgag200/mgag200_bmc.c
@@ -77,11 +77,6 @@ void mgag200_bmc_enable_vidrst(struct mga_device *mdev)
  {
        u8 tmp;

- /* Ensure that the vrsten and hrsten are set */

-       WREG8(MGAREG_CRTCEXT_INDEX, 1);
-       tmp = RREG8(MGAREG_CRTCEXT_DATA);
-       WREG8(MGAREG_CRTCEXT_DATA, tmp | 0x88);
-
        /* Assert rstlvl2 */
        WREG8(DAC_INDEX, MGA1064_REMHEADCTL2);
        tmp = RREG8(DAC_DATA);
@@ -108,6 +103,25 @@ void mgag200_bmc_enable_vidrst(struct mga_device *mdev)
        WREG_DAC(MGA1064_GEN_IO_DATA, tmp);
  }

+static int mgag200_bmc_encoder_helper_atomic_check(struct drm_encoder *encoder,

+                                                  struct drm_crtc_state 
*crtc_state,
+                                                  struct drm_connector_state 
*conn_state)
+{
+       struct mga_device *mdev = to_mga_device(encoder->dev);
+       struct mgag200_crtc_state *mgag200_crtc_state = 
to_mgag200_crtc_state(crtc_state);
+
+       if (mdev->info->has_vidrst)
+               mgag200_crtc_state->set_vidrst = true;
+       else
+               mgag200_crtc_state->set_vidrst = false;
+


I think you can simplify it with:

mgag200_crtc_state->set_vidrst = mdev->info->has_vidrst;

+       return 0;
+}
+
+static const struct drm_encoder_helper_funcs mgag200_bmc_encoder_helper_funcs 
= {
+       .atomic_check = mgag200_bmc_encoder_helper_atomic_check,
+};
+
  static const struct drm_encoder_funcs mgag200_bmc_encoder_funcs = {
        .destroy = drm_encoder_cleanup,
  };
@@ -190,6 +204,8 @@ int mgag200_bmc_output_init(struct mga_device *mdev, struct 
drm_connector *physi
                               DRM_MODE_ENCODER_VIRTUAL, NULL);
        if (ret)
                return ret;
+       drm_encoder_helper_add(encoder, &mgag200_bmc_encoder_helper_funcs);
+
        encoder->possible_crtcs = drm_crtc_mask(crtc);

bmc_connector = &mdev->output.bmc.bmc_connector;

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 7f7dfbd0f013..4b75613de882 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -179,6 +179,8 @@ struct mgag200_crtc_state {
        const struct drm_format_info *format;

struct mgag200_pll_values pixpllc;

+
+       bool set_vidrst;
  };

static inline struct mgag200_crtc_state *to_mgag200_crtc_state(struct drm_crtc_state *base)

@@ -430,7 +432,8 @@ void mgag200_crtc_atomic_destroy_state(struct drm_crtc 
*crtc, struct drm_crtc_st
        .atomic_duplicate_state = mgag200_crtc_atomic_duplicate_state, \
        .atomic_destroy_state = mgag200_crtc_atomic_destroy_state

-void mgag200_set_mode_regs(struct mga_device *mdev, const struct drm_display_mode *mode);

+void mgag200_set_mode_regs(struct mga_device *mdev, const struct 
drm_display_mode *mode,
+                          bool set_vidrst);
  void mgag200_set_format_regs(struct mga_device *mdev, const struct 
drm_format_info *format);
  void mgag200_enable_display(struct mga_device *mdev);
  void mgag200_init_registers(struct mga_device *mdev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c 
b/drivers/gpu/drm/mgag200/mgag200_g200er.c
index 4e8a1756138d..abfbed6ec390 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
@@ -195,7 +195,7 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct 
drm_crtc *crtc,
                funcs->disable_vidrst(mdev);

mgag200_set_format_regs(mdev, format);

-       mgag200_set_mode_regs(mdev, adjusted_mode);
+       mgag200_set_mode_regs(mdev, adjusted_mode, 
mgag200_crtc_state->set_vidrst);

if (funcs->pixpllc_atomic_update)

                funcs->pixpllc_atomic_update(crtc, old_state);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c 
b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
index d884f3cb0ec7..acc99999e2b5 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
@@ -196,7 +196,7 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct 
drm_crtc *crtc,
                funcs->disable_vidrst(mdev);

mgag200_set_format_regs(mdev, format);

-       mgag200_set_mode_regs(mdev, adjusted_mode);
+       mgag200_set_mode_regs(mdev, adjusted_mode, 
mgag200_crtc_state->set_vidrst);

if (funcs->pixpllc_atomic_update)

                funcs->pixpllc_atomic_update(crtc, old_state);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c 
b/drivers/gpu/drm/mgag200/mgag200_g200se.c
index a824bb8ad579..be4e124102c6 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
@@ -327,7 +327,7 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct 
drm_crtc *crtc,
                funcs->disable_vidrst(mdev);

mgag200_set_format_regs(mdev, format);

-       mgag200_set_mode_regs(mdev, adjusted_mode);
+       mgag200_set_mode_regs(mdev, adjusted_mode, 
mgag200_crtc_state->set_vidrst);

if (funcs->pixpllc_atomic_update)

                funcs->pixpllc_atomic_update(crtc, old_state);
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index bb6204002cb3..4f4612192e30 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -201,9 +201,9 @@ void mgag200_init_registers(struct mga_device *mdev)
        WREG8(MGA_MISC_OUT, misc);
  }

-void mgag200_set_mode_regs(struct mga_device *mdev, const struct drm_display_mode *mode)

+void mgag200_set_mode_regs(struct mga_device *mdev, const struct 
drm_display_mode *mode,
+                          bool set_vidrst)
  {
-       const struct mgag200_device_info *info = mdev->info;
        unsigned int hdisplay, hsyncstart, hsyncend, htotal;
        unsigned int vdisplay, vsyncstart, vsyncend, vtotal;
        u8 misc, crtcext1, crtcext2, crtcext5;
@@ -238,9 +238,11 @@ void mgag200_set_mode_regs(struct mga_device *mdev, const 
struct drm_display_mod
                   ((hdisplay & 0x100) >> 7) |
                   ((hsyncstart & 0x100) >> 6) |
                    (htotal & 0x40);
-       if (info->has_vidrst)
+       if (set_vidrst)
                crtcext1 |= MGAREG_CRTCEXT1_VRSTEN |
                            MGAREG_CRTCEXT1_HRSTEN;
+       else
+               crtcext1 &= ~(MGAREG_CRTCEXT1_VRSTEN | MGAREG_CRTCEXT1_HRSTEN);

crtcext2 = ((vtotal & 0xc00) >> 10) |

                   ((vdisplay & 0x400) >> 8) |
@@ -656,7 +658,7 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc 
*crtc, struct drm_atomic_
                funcs->disable_vidrst(mdev);

mgag200_set_format_regs(mdev, format);

-       mgag200_set_mode_regs(mdev, adjusted_mode);
+       mgag200_set_mode_regs(mdev, adjusted_mode, 
mgag200_crtc_state->set_vidrst);

if (funcs->pixpllc_atomic_update)

                funcs->pixpllc_atomic_update(crtc, old_state);
@@ -717,6 +719,7 @@ struct drm_crtc_state 
*mgag200_crtc_atomic_duplicate_state(struct drm_crtc *crtc
        new_mgag200_crtc_state->format = mgag200_crtc_state->format;
        memcpy(&new_mgag200_crtc_state->pixpllc, &mgag200_crtc_state->pixpllc,
               sizeof(new_mgag200_crtc_state->pixpllc));
+       new_mgag200_crtc_state->set_vidrst = mgag200_crtc_state->set_vidrst;

return &new_mgag200_crtc_state->base;


With the small nitpick.

Reviewed-by: Jocelyn Falempe <jfale...@redhat.com>


--

Jocelyn

Re: [PATCH 1/4] drm/mgag200: Only set VIDRST bits in CRTC modesetting

Reply via email to