* [PATCH v2 1/3] drm/mgag200: Only set VIDRST bits in CRTC modesetting
2024-07-10 8:42 [PATCH v2 0/3] drm/mgag200: Control VIDRST and BMC from CRTC Thomas Zimmermann
@ 2024-07-10 8:42 ` Thomas Zimmermann
2024-07-10 14:25 ` Jocelyn Falempe
2024-07-10 8:42 ` [PATCH v2 2/3] drm/mgag200: Remove vidrst callbacks from struct mgag200_device_funcs Thomas Zimmermann
2024-07-10 8:42 ` [PATCH v2 3/3] drm/mgag200: Rename BMC vidrst names Thomas Zimmermann
2 siblings, 1 reply; 8+ messages in thread
From: Thomas Zimmermann @ 2024-07-10 8:42 UTC (permalink / raw)
To: jfalempe, airlied, maarten.lankhorst, mripard, airlied, daniel
Cc: dri-devel, Thomas Zimmermann
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.
Only set VRSTEN and HRSTEN bits in the CRTC mode-setting code, so the
bits are independent 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 CRTC's atomic_check helper.
v2:
- keep logic entirely in CRTC (Jocelyn)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/mgag200/mgag200_bmc.c | 5 -----
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 | 14 ++++++++++----
6 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_bmc.c b/drivers/gpu/drm/mgag200/mgag200_bmc.c
index 23ef85aa7e37..1c7aa4f36787 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);
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 d4550e4b3b01..e746f365fecf 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) |
@@ -597,6 +599,7 @@ int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_st
struct mga_device *mdev = to_mga_device(dev);
const struct mgag200_device_funcs *funcs = mdev->funcs;
struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(new_state, crtc);
+ struct mgag200_crtc_state *new_mgag200_crtc_state = to_mgag200_crtc_state(new_crtc_state);
struct drm_property_blob *new_gamma_lut = new_crtc_state->gamma_lut;
int ret;
@@ -607,6 +610,8 @@ int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_st
if (ret)
return ret;
+ new_mgag200_crtc_state->set_vidrst = mdev->info->has_vidrst;
+
if (new_crtc_state->mode_changed) {
if (funcs->pixpllc_atomic_check) {
ret = funcs->pixpllc_atomic_check(crtc, new_state);
@@ -656,7 +661,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 +722,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;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/3] drm/mgag200: Only set VIDRST bits in CRTC modesetting
2024-07-10 8:42 ` [PATCH v2 1/3] drm/mgag200: Only set VIDRST bits in CRTC modesetting Thomas Zimmermann
@ 2024-07-10 14:25 ` Jocelyn Falempe
2024-07-10 14:45 ` Thomas Zimmermann
0 siblings, 1 reply; 8+ messages in thread
From: Jocelyn Falempe @ 2024-07-10 14:25 UTC (permalink / raw)
To: Thomas Zimmermann, airlied, maarten.lankhorst, mripard, airlied,
daniel
Cc: dri-devel
On 10/07/2024 10:42, 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.
>
> Only set VRSTEN and HRSTEN bits in the CRTC mode-setting code, so the
> bits are independent 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 CRTC's atomic_check helper.
Thanks, I think this has less chance for regression.
I've only a small nitpick below.
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
>
> v2:
> - keep logic entirely in CRTC (Jocelyn)
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/mgag200/mgag200_bmc.c | 5 -----
> 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 | 14 ++++++++++----
> 6 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_bmc.c b/drivers/gpu/drm/mgag200/mgag200_bmc.c
> index 23ef85aa7e37..1c7aa4f36787 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);
> 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 d4550e4b3b01..e746f365fecf 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);
The else case is useless, as crtcext1 has already this bits set to 0
unconditionnaly.
>
> crtcext2 = ((vtotal & 0xc00) >> 10) |
> ((vdisplay & 0x400) >> 8) |
> @@ -597,6 +599,7 @@ int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_st
> struct mga_device *mdev = to_mga_device(dev);
> const struct mgag200_device_funcs *funcs = mdev->funcs;
> struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(new_state, crtc);
> + struct mgag200_crtc_state *new_mgag200_crtc_state = to_mgag200_crtc_state(new_crtc_state);
> struct drm_property_blob *new_gamma_lut = new_crtc_state->gamma_lut;
> int ret;
>
> @@ -607,6 +610,8 @@ int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_st
> if (ret)
> return ret;
>
> + new_mgag200_crtc_state->set_vidrst = mdev->info->has_vidrst;
> +
> if (new_crtc_state->mode_changed) {
> if (funcs->pixpllc_atomic_check) {
> ret = funcs->pixpllc_atomic_check(crtc, new_state);
> @@ -656,7 +661,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 +722,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;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/3] drm/mgag200: Only set VIDRST bits in CRTC modesetting
2024-07-10 14:25 ` Jocelyn Falempe
@ 2024-07-10 14:45 ` Thomas Zimmermann
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Zimmermann @ 2024-07-10 14:45 UTC (permalink / raw)
To: Jocelyn Falempe, airlied, maarten.lankhorst, mripard, airlied,
daniel
Cc: dri-devel
Hi
Am 10.07.24 um 16:25 schrieb Jocelyn Falempe:
>
>
>> + if (set_vidrst)
>> crtcext1 |= MGAREG_CRTCEXT1_VRSTEN |
>> MGAREG_CRTCEXT1_HRSTEN;
>> + else
>> + crtcext1 &= ~(MGAREG_CRTCEXT1_VRSTEN | MGAREG_CRTCEXT1_HRSTEN);
>
> The else case is useless, as crtcext1 has already this bits set to 0
> unconditionnaly.
Indeed. Will be fixed.
Best regards
Thomas
>
>> crtcext2 = ((vtotal & 0xc00) >> 10) |
>> ((vdisplay & 0x400) >> 8) |
>> @@ -597,6 +599,7 @@ int mgag200_crtc_helper_atomic_check(struct
>> drm_crtc *crtc, struct drm_atomic_st
>> struct mga_device *mdev = to_mga_device(dev);
>> const struct mgag200_device_funcs *funcs = mdev->funcs;
>> struct drm_crtc_state *new_crtc_state =
>> drm_atomic_get_new_crtc_state(new_state, crtc);
>> + struct mgag200_crtc_state *new_mgag200_crtc_state =
>> to_mgag200_crtc_state(new_crtc_state);
>> struct drm_property_blob *new_gamma_lut =
>> new_crtc_state->gamma_lut;
>> int ret;
>> @@ -607,6 +610,8 @@ int mgag200_crtc_helper_atomic_check(struct
>> drm_crtc *crtc, struct drm_atomic_st
>> if (ret)
>> return ret;
>> + new_mgag200_crtc_state->set_vidrst = mdev->info->has_vidrst;
>> +
>> if (new_crtc_state->mode_changed) {
>> if (funcs->pixpllc_atomic_check) {
>> ret = funcs->pixpllc_atomic_check(crtc, new_state);
>> @@ -656,7 +661,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 +722,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;
>> }
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] drm/mgag200: Remove vidrst callbacks from struct mgag200_device_funcs
2024-07-10 8:42 [PATCH v2 0/3] drm/mgag200: Control VIDRST and BMC from CRTC Thomas Zimmermann
2024-07-10 8:42 ` [PATCH v2 1/3] drm/mgag200: Only set VIDRST bits in CRTC modesetting Thomas Zimmermann
@ 2024-07-10 8:42 ` Thomas Zimmermann
2024-07-10 14:26 ` Jocelyn Falempe
2024-07-10 8:42 ` [PATCH v2 3/3] drm/mgag200: Rename BMC vidrst names Thomas Zimmermann
2 siblings, 1 reply; 8+ messages in thread
From: Thomas Zimmermann @ 2024-07-10 8:42 UTC (permalink / raw)
To: jfalempe, airlied, maarten.lankhorst, mripard, airlied, daniel
Cc: dri-devel, Thomas Zimmermann
The callbacks disable_vidrst and enable_vidrst are obsolete. Remove
the fields from struct mgag200_device_funcs. Instead call their
implementations directly of the field 'has_vidrst' has been set in
struct mgag200_device_info.
Also change the logic slightly. The BMC used to start and stop scanout
during the CRTC's atomic_enable and atomic_disable. Plane updates were
done while the BMC scanned out the display. Now only stop once in
atomic_disable at the beginning of a modeset and then restart the
scanout at the end of a modeset in atomic_enable. While the modeset
takes place, the BMC does not scanout at all.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/mgag200/mgag200_drv.h | 12 ------------
drivers/gpu/drm/mgag200/mgag200_g200er.c | 7 ++-----
drivers/gpu/drm/mgag200/mgag200_g200ev.c | 7 ++-----
drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 2 --
drivers/gpu/drm/mgag200/mgag200_g200se.c | 7 ++-----
drivers/gpu/drm/mgag200/mgag200_g200wb.c | 2 --
drivers/gpu/drm/mgag200/mgag200_mode.c | 15 ++++-----------
7 files changed, 10 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 4b75613de882..4a46c8c006c8 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -247,18 +247,6 @@ struct mgag200_device_info {
}
struct mgag200_device_funcs {
- /*
- * Disables an external reset source (i.e., BMC) before programming
- * a new display mode.
- */
- void (*disable_vidrst)(struct mga_device *mdev);
-
- /*
- * Enables an external reset source (i.e., BMC) after programming
- * a new display mode.
- */
- void (*enable_vidrst)(struct mga_device *mdev);
-
/*
* Validate that the given state can be programmed into PIXPLLC. On
* success, the calculated parameters should be stored in the CRTC's
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
index abfbed6ec390..b3bb3e9fb0d1 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
@@ -191,9 +191,6 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc,
struct mgag200_crtc_state *mgag200_crtc_state = to_mgag200_crtc_state(crtc_state);
const struct drm_format_info *format = mgag200_crtc_state->format;
- if (funcs->disable_vidrst)
- funcs->disable_vidrst(mdev);
-
mgag200_set_format_regs(mdev, format);
mgag200_set_mode_regs(mdev, adjusted_mode, mgag200_crtc_state->set_vidrst);
@@ -209,8 +206,8 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc,
mgag200_enable_display(mdev);
- if (funcs->enable_vidrst)
- funcs->enable_vidrst(mdev);
+ if (mdev->info->has_vidrst)
+ mgag200_bmc_enable_vidrst(mdev);
}
static const struct drm_crtc_helper_funcs mgag200_g200er_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
index acc99999e2b5..3ac0a508e2c5 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
@@ -192,9 +192,6 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc,
struct mgag200_crtc_state *mgag200_crtc_state = to_mgag200_crtc_state(crtc_state);
const struct drm_format_info *format = mgag200_crtc_state->format;
- if (funcs->disable_vidrst)
- funcs->disable_vidrst(mdev);
-
mgag200_set_format_regs(mdev, format);
mgag200_set_mode_regs(mdev, adjusted_mode, mgag200_crtc_state->set_vidrst);
@@ -210,8 +207,8 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc,
mgag200_enable_display(mdev);
- if (funcs->enable_vidrst)
- funcs->enable_vidrst(mdev);
+ if (mdev->info->has_vidrst)
+ mgag200_bmc_enable_vidrst(mdev);
}
static const struct drm_crtc_helper_funcs mgag200_g200ev_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
index 839401e8b465..265f3e95830a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
@@ -146,8 +146,6 @@ static const struct mgag200_device_info mgag200_g200ew3_device_info =
MGAG200_DEVICE_INFO_INIT(2048, 2048, 0, true, 0, 1, false);
static const struct mgag200_device_funcs mgag200_g200ew3_device_funcs = {
- .disable_vidrst = mgag200_bmc_disable_vidrst,
- .enable_vidrst = mgag200_bmc_enable_vidrst,
.pixpllc_atomic_check = mgag200_g200ew3_pixpllc_atomic_check,
.pixpllc_atomic_update = mgag200_g200wb_pixpllc_atomic_update, // same as G200WB
};
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c
index be4e124102c6..7a8099eb100c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
@@ -323,9 +323,6 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc,
struct mgag200_crtc_state *mgag200_crtc_state = to_mgag200_crtc_state(crtc_state);
const struct drm_format_info *format = mgag200_crtc_state->format;
- if (funcs->disable_vidrst)
- funcs->disable_vidrst(mdev);
-
mgag200_set_format_regs(mdev, format);
mgag200_set_mode_regs(mdev, adjusted_mode, mgag200_crtc_state->set_vidrst);
@@ -341,8 +338,8 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc,
mgag200_enable_display(mdev);
- if (funcs->enable_vidrst)
- funcs->enable_vidrst(mdev);
+ if (mdev->info->has_vidrst)
+ mgag200_bmc_enable_vidrst(mdev);
}
static const struct drm_crtc_helper_funcs mgag200_g200se_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200wb.c b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
index 835df0f4fc13..e25477347c3e 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200wb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
@@ -280,8 +280,6 @@ static const struct mgag200_device_info mgag200_g200wb_device_info =
MGAG200_DEVICE_INFO_INIT(1280, 1024, 31877, true, 0, 1, false);
static const struct mgag200_device_funcs mgag200_g200wb_device_funcs = {
- .disable_vidrst = mgag200_bmc_disable_vidrst,
- .enable_vidrst = mgag200_bmc_enable_vidrst,
.pixpllc_atomic_check = mgag200_g200wb_pixpllc_atomic_check,
.pixpllc_atomic_update = mgag200_g200wb_pixpllc_atomic_update,
};
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index e746f365fecf..fcc10723d385 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -657,9 +657,6 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_
struct mgag200_crtc_state *mgag200_crtc_state = to_mgag200_crtc_state(crtc_state);
const struct drm_format_info *format = mgag200_crtc_state->format;
- if (funcs->disable_vidrst)
- funcs->disable_vidrst(mdev);
-
mgag200_set_format_regs(mdev, format);
mgag200_set_mode_regs(mdev, adjusted_mode, mgag200_crtc_state->set_vidrst);
@@ -673,22 +670,18 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_
mgag200_enable_display(mdev);
- if (funcs->enable_vidrst)
- funcs->enable_vidrst(mdev);
+ if (mdev->info->has_vidrst)
+ mgag200_bmc_enable_vidrst(mdev);
}
void mgag200_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *old_state)
{
struct mga_device *mdev = to_mga_device(crtc->dev);
- const struct mgag200_device_funcs *funcs = mdev->funcs;
- if (funcs->disable_vidrst)
- funcs->disable_vidrst(mdev);
+ if (mdev->info->has_vidrst)
+ mgag200_bmc_disable_vidrst(mdev);
mgag200_disable_display(mdev);
-
- if (funcs->enable_vidrst)
- funcs->enable_vidrst(mdev);
}
void mgag200_crtc_reset(struct drm_crtc *crtc)
--
2.45.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 2/3] drm/mgag200: Remove vidrst callbacks from struct mgag200_device_funcs
2024-07-10 8:42 ` [PATCH v2 2/3] drm/mgag200: Remove vidrst callbacks from struct mgag200_device_funcs Thomas Zimmermann
@ 2024-07-10 14:26 ` Jocelyn Falempe
0 siblings, 0 replies; 8+ messages in thread
From: Jocelyn Falempe @ 2024-07-10 14:26 UTC (permalink / raw)
To: Thomas Zimmermann, airlied, maarten.lankhorst, mripard, airlied,
daniel
Cc: dri-devel
On 10/07/2024 10:42, Thomas Zimmermann wrote:
> The callbacks disable_vidrst and enable_vidrst are obsolete. Remove
> the fields from struct mgag200_device_funcs. Instead call their
> implementations directly of the field 'has_vidrst' has been set in
> struct mgag200_device_info.
>
> Also change the logic slightly. The BMC used to start and stop scanout
> during the CRTC's atomic_enable and atomic_disable. Plane updates were
> done while the BMC scanned out the display. Now only stop once in
> atomic_disable at the beginning of a modeset and then restart the
> scanout at the end of a modeset in atomic_enable. While the modeset
> takes place, the BMC does not scanout at all.
Thanks, it looks good to me.
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/mgag200/mgag200_drv.h | 12 ------------
> drivers/gpu/drm/mgag200/mgag200_g200er.c | 7 ++-----
> drivers/gpu/drm/mgag200/mgag200_g200ev.c | 7 ++-----
> drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 2 --
> drivers/gpu/drm/mgag200/mgag200_g200se.c | 7 ++-----
> drivers/gpu/drm/mgag200/mgag200_g200wb.c | 2 --
> drivers/gpu/drm/mgag200/mgag200_mode.c | 15 ++++-----------
> 7 files changed, 10 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 4b75613de882..4a46c8c006c8 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -247,18 +247,6 @@ struct mgag200_device_info {
> }
>
> struct mgag200_device_funcs {
> - /*
> - * Disables an external reset source (i.e., BMC) before programming
> - * a new display mode.
> - */
> - void (*disable_vidrst)(struct mga_device *mdev);
> -
> - /*
> - * Enables an external reset source (i.e., BMC) after programming
> - * a new display mode.
> - */
> - void (*enable_vidrst)(struct mga_device *mdev);
> -
> /*
> * Validate that the given state can be programmed into PIXPLLC. On
> * success, the calculated parameters should be stored in the CRTC's
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> index abfbed6ec390..b3bb3e9fb0d1 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> @@ -191,9 +191,6 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc,
> struct mgag200_crtc_state *mgag200_crtc_state = to_mgag200_crtc_state(crtc_state);
> const struct drm_format_info *format = mgag200_crtc_state->format;
>
> - if (funcs->disable_vidrst)
> - funcs->disable_vidrst(mdev);
> -
> mgag200_set_format_regs(mdev, format);
> mgag200_set_mode_regs(mdev, adjusted_mode, mgag200_crtc_state->set_vidrst);
>
> @@ -209,8 +206,8 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>
> mgag200_enable_display(mdev);
>
> - if (funcs->enable_vidrst)
> - funcs->enable_vidrst(mdev);
> + if (mdev->info->has_vidrst)
> + mgag200_bmc_enable_vidrst(mdev);
> }
>
> static const struct drm_crtc_helper_funcs mgag200_g200er_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> index acc99999e2b5..3ac0a508e2c5 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> @@ -192,9 +192,6 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc,
> struct mgag200_crtc_state *mgag200_crtc_state = to_mgag200_crtc_state(crtc_state);
> const struct drm_format_info *format = mgag200_crtc_state->format;
>
> - if (funcs->disable_vidrst)
> - funcs->disable_vidrst(mdev);
> -
> mgag200_set_format_regs(mdev, format);
> mgag200_set_mode_regs(mdev, adjusted_mode, mgag200_crtc_state->set_vidrst);
>
> @@ -210,8 +207,8 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>
> mgag200_enable_display(mdev);
>
> - if (funcs->enable_vidrst)
> - funcs->enable_vidrst(mdev);
> + if (mdev->info->has_vidrst)
> + mgag200_bmc_enable_vidrst(mdev);
> }
>
> static const struct drm_crtc_helper_funcs mgag200_g200ev_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
> index 839401e8b465..265f3e95830a 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
> @@ -146,8 +146,6 @@ static const struct mgag200_device_info mgag200_g200ew3_device_info =
> MGAG200_DEVICE_INFO_INIT(2048, 2048, 0, true, 0, 1, false);
>
> static const struct mgag200_device_funcs mgag200_g200ew3_device_funcs = {
> - .disable_vidrst = mgag200_bmc_disable_vidrst,
> - .enable_vidrst = mgag200_bmc_enable_vidrst,
> .pixpllc_atomic_check = mgag200_g200ew3_pixpllc_atomic_check,
> .pixpllc_atomic_update = mgag200_g200wb_pixpllc_atomic_update, // same as G200WB
> };
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> index be4e124102c6..7a8099eb100c 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> @@ -323,9 +323,6 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc,
> struct mgag200_crtc_state *mgag200_crtc_state = to_mgag200_crtc_state(crtc_state);
> const struct drm_format_info *format = mgag200_crtc_state->format;
>
> - if (funcs->disable_vidrst)
> - funcs->disable_vidrst(mdev);
> -
> mgag200_set_format_regs(mdev, format);
> mgag200_set_mode_regs(mdev, adjusted_mode, mgag200_crtc_state->set_vidrst);
>
> @@ -341,8 +338,8 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>
> mgag200_enable_display(mdev);
>
> - if (funcs->enable_vidrst)
> - funcs->enable_vidrst(mdev);
> + if (mdev->info->has_vidrst)
> + mgag200_bmc_enable_vidrst(mdev);
> }
>
> static const struct drm_crtc_helper_funcs mgag200_g200se_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200wb.c b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
> index 835df0f4fc13..e25477347c3e 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200wb.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
> @@ -280,8 +280,6 @@ static const struct mgag200_device_info mgag200_g200wb_device_info =
> MGAG200_DEVICE_INFO_INIT(1280, 1024, 31877, true, 0, 1, false);
>
> static const struct mgag200_device_funcs mgag200_g200wb_device_funcs = {
> - .disable_vidrst = mgag200_bmc_disable_vidrst,
> - .enable_vidrst = mgag200_bmc_enable_vidrst,
> .pixpllc_atomic_check = mgag200_g200wb_pixpllc_atomic_check,
> .pixpllc_atomic_update = mgag200_g200wb_pixpllc_atomic_update,
> };
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index e746f365fecf..fcc10723d385 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -657,9 +657,6 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_
> struct mgag200_crtc_state *mgag200_crtc_state = to_mgag200_crtc_state(crtc_state);
> const struct drm_format_info *format = mgag200_crtc_state->format;
>
> - if (funcs->disable_vidrst)
> - funcs->disable_vidrst(mdev);
> -
> mgag200_set_format_regs(mdev, format);
> mgag200_set_mode_regs(mdev, adjusted_mode, mgag200_crtc_state->set_vidrst);
>
> @@ -673,22 +670,18 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_
>
> mgag200_enable_display(mdev);
>
> - if (funcs->enable_vidrst)
> - funcs->enable_vidrst(mdev);
> + if (mdev->info->has_vidrst)
> + mgag200_bmc_enable_vidrst(mdev);
> }
>
> void mgag200_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *old_state)
> {
> struct mga_device *mdev = to_mga_device(crtc->dev);
> - const struct mgag200_device_funcs *funcs = mdev->funcs;
>
> - if (funcs->disable_vidrst)
> - funcs->disable_vidrst(mdev);
> + if (mdev->info->has_vidrst)
> + mgag200_bmc_disable_vidrst(mdev);
>
> mgag200_disable_display(mdev);
> -
> - if (funcs->enable_vidrst)
> - funcs->enable_vidrst(mdev);
> }
>
> void mgag200_crtc_reset(struct drm_crtc *crtc)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] drm/mgag200: Rename BMC vidrst names
2024-07-10 8:42 [PATCH v2 0/3] drm/mgag200: Control VIDRST and BMC from CRTC Thomas Zimmermann
2024-07-10 8:42 ` [PATCH v2 1/3] drm/mgag200: Only set VIDRST bits in CRTC modesetting Thomas Zimmermann
2024-07-10 8:42 ` [PATCH v2 2/3] drm/mgag200: Remove vidrst callbacks from struct mgag200_device_funcs Thomas Zimmermann
@ 2024-07-10 8:42 ` Thomas Zimmermann
2024-07-10 14:27 ` Jocelyn Falempe
2 siblings, 1 reply; 8+ messages in thread
From: Thomas Zimmermann @ 2024-07-10 8:42 UTC (permalink / raw)
To: jfalempe, airlied, maarten.lankhorst, mripard, airlied, daniel
Cc: dri-devel, Thomas Zimmermann
The BMC's scanout synchronization is only indirectly related to the
VIDRST functionality. Do some renaming.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/mgag200/mgag200_bmc.c | 4 ++--
drivers/gpu/drm/mgag200/mgag200_drv.h | 14 +++++++-------
drivers/gpu/drm/mgag200/mgag200_g200er.c | 4 ++--
drivers/gpu/drm/mgag200/mgag200_g200ev.c | 4 ++--
drivers/gpu/drm/mgag200/mgag200_g200se.c | 4 ++--
drivers/gpu/drm/mgag200/mgag200_mode.c | 10 +++++-----
6 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_bmc.c b/drivers/gpu/drm/mgag200/mgag200_bmc.c
index 1c7aa4f36787..45e35dffb3ea 100644
--- a/drivers/gpu/drm/mgag200/mgag200_bmc.c
+++ b/drivers/gpu/drm/mgag200/mgag200_bmc.c
@@ -14,7 +14,7 @@ static struct mgag200_bmc_connector *to_mgag200_bmc_connector(struct drm_connect
return container_of(connector, struct mgag200_bmc_connector, base);
}
-void mgag200_bmc_disable_vidrst(struct mga_device *mdev)
+void mgag200_bmc_stop_scanout(struct mga_device *mdev)
{
u8 tmp;
int iter_max;
@@ -73,7 +73,7 @@ void mgag200_bmc_disable_vidrst(struct mga_device *mdev)
}
}
-void mgag200_bmc_enable_vidrst(struct mga_device *mdev)
+void mgag200_bmc_start_scanout(struct mga_device *mdev)
{
u8 tmp;
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 4a46c8c006c8..f97eaa49b089 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -216,8 +216,8 @@ struct mgag200_device_info {
*/
unsigned long max_mem_bandwidth;
- /* HW has external source (e.g., BMC) to synchronize with */
- bool has_vidrst:1;
+ /* Synchronize scanout with BMC */
+ bool sync_bmc:1;
struct {
unsigned data_bit:3;
@@ -232,13 +232,13 @@ struct mgag200_device_info {
};
#define MGAG200_DEVICE_INFO_INIT(_max_hdisplay, _max_vdisplay, _max_mem_bandwidth, \
- _has_vidrst, _i2c_data_bit, _i2c_clock_bit, \
+ _sync_bmc, _i2c_data_bit, _i2c_clock_bit, \
_bug_no_startadd) \
{ \
.max_hdisplay = (_max_hdisplay), \
.max_vdisplay = (_max_vdisplay), \
.max_mem_bandwidth = (_max_mem_bandwidth), \
- .has_vidrst = (_has_vidrst), \
+ .sync_bmc = (_sync_bmc), \
.i2c = { \
.data_bit = (_i2c_data_bit), \
.clock_bit = (_i2c_clock_bit), \
@@ -430,9 +430,9 @@ int mgag200_mode_config_init(struct mga_device *mdev, resource_size_t vram_avail
/* mgag200_vga.c */
int mgag200_vga_output_init(struct mga_device *mdev);
- /* mgag200_bmc.c */
-void mgag200_bmc_disable_vidrst(struct mga_device *mdev);
-void mgag200_bmc_enable_vidrst(struct mga_device *mdev);
+/* mgag200_bmc.c */
+void mgag200_bmc_stop_scanout(struct mga_device *mdev);
+void mgag200_bmc_start_scanout(struct mga_device *mdev);
int mgag200_bmc_output_init(struct mga_device *mdev, struct drm_connector *physical_connector);
#endif /* __MGAG200_DRV_H__ */
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
index b3bb3e9fb0d1..737a48aa9160 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
@@ -206,8 +206,8 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc,
mgag200_enable_display(mdev);
- if (mdev->info->has_vidrst)
- mgag200_bmc_enable_vidrst(mdev);
+ if (mdev->info->sync_bmc)
+ mgag200_bmc_start_scanout(mdev);
}
static const struct drm_crtc_helper_funcs mgag200_g200er_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
index 3ac0a508e2c5..8d1ccc2ad94a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
@@ -207,8 +207,8 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc,
mgag200_enable_display(mdev);
- if (mdev->info->has_vidrst)
- mgag200_bmc_enable_vidrst(mdev);
+ if (mdev->info->sync_bmc)
+ mgag200_bmc_start_scanout(mdev);
}
static const struct drm_crtc_helper_funcs mgag200_g200ev_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c
index 7a8099eb100c..cf7f6897838f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
@@ -338,8 +338,8 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc,
mgag200_enable_display(mdev);
- if (mdev->info->has_vidrst)
- mgag200_bmc_enable_vidrst(mdev);
+ if (mdev->info->sync_bmc)
+ mgag200_bmc_start_scanout(mdev);
}
static const struct drm_crtc_helper_funcs mgag200_g200se_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index fcc10723d385..735eb5906892 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -610,7 +610,7 @@ int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_st
if (ret)
return ret;
- new_mgag200_crtc_state->set_vidrst = mdev->info->has_vidrst;
+ new_mgag200_crtc_state->set_vidrst = mdev->info->sync_bmc;
if (new_crtc_state->mode_changed) {
if (funcs->pixpllc_atomic_check) {
@@ -670,16 +670,16 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_
mgag200_enable_display(mdev);
- if (mdev->info->has_vidrst)
- mgag200_bmc_enable_vidrst(mdev);
+ if (mdev->info->sync_bmc)
+ mgag200_bmc_start_scanout(mdev);
}
void mgag200_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *old_state)
{
struct mga_device *mdev = to_mga_device(crtc->dev);
- if (mdev->info->has_vidrst)
- mgag200_bmc_disable_vidrst(mdev);
+ if (mdev->info->sync_bmc)
+ mgag200_bmc_stop_scanout(mdev);
mgag200_disable_display(mdev);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 3/3] drm/mgag200: Rename BMC vidrst names
2024-07-10 8:42 ` [PATCH v2 3/3] drm/mgag200: Rename BMC vidrst names Thomas Zimmermann
@ 2024-07-10 14:27 ` Jocelyn Falempe
0 siblings, 0 replies; 8+ messages in thread
From: Jocelyn Falempe @ 2024-07-10 14:27 UTC (permalink / raw)
To: Thomas Zimmermann, airlied, maarten.lankhorst, mripard, airlied,
daniel
Cc: dri-devel
On 10/07/2024 10:42, Thomas Zimmermann wrote:
> The BMC's scanout synchronization is only indirectly related to the
> VIDRST functionality. Do some renaming.
Thanks, it looks good to me.
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/mgag200/mgag200_bmc.c | 4 ++--
> drivers/gpu/drm/mgag200/mgag200_drv.h | 14 +++++++-------
> drivers/gpu/drm/mgag200/mgag200_g200er.c | 4 ++--
> drivers/gpu/drm/mgag200/mgag200_g200ev.c | 4 ++--
> drivers/gpu/drm/mgag200/mgag200_g200se.c | 4 ++--
> drivers/gpu/drm/mgag200/mgag200_mode.c | 10 +++++-----
> 6 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_bmc.c b/drivers/gpu/drm/mgag200/mgag200_bmc.c
> index 1c7aa4f36787..45e35dffb3ea 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_bmc.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_bmc.c
> @@ -14,7 +14,7 @@ static struct mgag200_bmc_connector *to_mgag200_bmc_connector(struct drm_connect
> return container_of(connector, struct mgag200_bmc_connector, base);
> }
>
> -void mgag200_bmc_disable_vidrst(struct mga_device *mdev)
> +void mgag200_bmc_stop_scanout(struct mga_device *mdev)
> {
> u8 tmp;
> int iter_max;
> @@ -73,7 +73,7 @@ void mgag200_bmc_disable_vidrst(struct mga_device *mdev)
> }
> }
>
> -void mgag200_bmc_enable_vidrst(struct mga_device *mdev)
> +void mgag200_bmc_start_scanout(struct mga_device *mdev)
> {
> u8 tmp;
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 4a46c8c006c8..f97eaa49b089 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -216,8 +216,8 @@ struct mgag200_device_info {
> */
> unsigned long max_mem_bandwidth;
>
> - /* HW has external source (e.g., BMC) to synchronize with */
> - bool has_vidrst:1;
> + /* Synchronize scanout with BMC */
> + bool sync_bmc:1;
>
> struct {
> unsigned data_bit:3;
> @@ -232,13 +232,13 @@ struct mgag200_device_info {
> };
>
> #define MGAG200_DEVICE_INFO_INIT(_max_hdisplay, _max_vdisplay, _max_mem_bandwidth, \
> - _has_vidrst, _i2c_data_bit, _i2c_clock_bit, \
> + _sync_bmc, _i2c_data_bit, _i2c_clock_bit, \
> _bug_no_startadd) \
> { \
> .max_hdisplay = (_max_hdisplay), \
> .max_vdisplay = (_max_vdisplay), \
> .max_mem_bandwidth = (_max_mem_bandwidth), \
> - .has_vidrst = (_has_vidrst), \
> + .sync_bmc = (_sync_bmc), \
> .i2c = { \
> .data_bit = (_i2c_data_bit), \
> .clock_bit = (_i2c_clock_bit), \
> @@ -430,9 +430,9 @@ int mgag200_mode_config_init(struct mga_device *mdev, resource_size_t vram_avail
> /* mgag200_vga.c */
> int mgag200_vga_output_init(struct mga_device *mdev);
>
> - /* mgag200_bmc.c */
> -void mgag200_bmc_disable_vidrst(struct mga_device *mdev);
> -void mgag200_bmc_enable_vidrst(struct mga_device *mdev);
> +/* mgag200_bmc.c */
> +void mgag200_bmc_stop_scanout(struct mga_device *mdev);
> +void mgag200_bmc_start_scanout(struct mga_device *mdev);
> int mgag200_bmc_output_init(struct mga_device *mdev, struct drm_connector *physical_connector);
>
> #endif /* __MGAG200_DRV_H__ */
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> index b3bb3e9fb0d1..737a48aa9160 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> @@ -206,8 +206,8 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>
> mgag200_enable_display(mdev);
>
> - if (mdev->info->has_vidrst)
> - mgag200_bmc_enable_vidrst(mdev);
> + if (mdev->info->sync_bmc)
> + mgag200_bmc_start_scanout(mdev);
> }
>
> static const struct drm_crtc_helper_funcs mgag200_g200er_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> index 3ac0a508e2c5..8d1ccc2ad94a 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> @@ -207,8 +207,8 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>
> mgag200_enable_display(mdev);
>
> - if (mdev->info->has_vidrst)
> - mgag200_bmc_enable_vidrst(mdev);
> + if (mdev->info->sync_bmc)
> + mgag200_bmc_start_scanout(mdev);
> }
>
> static const struct drm_crtc_helper_funcs mgag200_g200ev_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> index 7a8099eb100c..cf7f6897838f 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> @@ -338,8 +338,8 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>
> mgag200_enable_display(mdev);
>
> - if (mdev->info->has_vidrst)
> - mgag200_bmc_enable_vidrst(mdev);
> + if (mdev->info->sync_bmc)
> + mgag200_bmc_start_scanout(mdev);
> }
>
> static const struct drm_crtc_helper_funcs mgag200_g200se_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index fcc10723d385..735eb5906892 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -610,7 +610,7 @@ int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_st
> if (ret)
> return ret;
>
> - new_mgag200_crtc_state->set_vidrst = mdev->info->has_vidrst;
> + new_mgag200_crtc_state->set_vidrst = mdev->info->sync_bmc;
>
> if (new_crtc_state->mode_changed) {
> if (funcs->pixpllc_atomic_check) {
> @@ -670,16 +670,16 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_
>
> mgag200_enable_display(mdev);
>
> - if (mdev->info->has_vidrst)
> - mgag200_bmc_enable_vidrst(mdev);
> + if (mdev->info->sync_bmc)
> + mgag200_bmc_start_scanout(mdev);
> }
>
> void mgag200_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *old_state)
> {
> struct mga_device *mdev = to_mga_device(crtc->dev);
>
> - if (mdev->info->has_vidrst)
> - mgag200_bmc_disable_vidrst(mdev);
> + if (mdev->info->sync_bmc)
> + mgag200_bmc_stop_scanout(mdev);
>
> mgag200_disable_display(mdev);
> }
^ permalink raw reply [flat|nested] 8+ messages in thread