| Message ID | 20231214163849.359691-1-jfalempe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/mgag200: Fix gamma lut not initialized for G200ER, G200EV, G200SE | expand |
Hi Am 14.12.23 um 17:38 schrieb Jocelyn Falempe: > When mgag200 switched from simple KMS to regular atomic helpers, > the initialization of the gamma settings was lost. > This leads to a black screen, if the bios/uefi doesn't use the same > pixel color depth. > This has been fixed with commit ad81e23426a6 ("drm/mgag200: Fix gamma > lut not initialized.") for most G200, but G200ER, G200EV, G200SE use > their own version of crtc_helper_atomic_enable() and need to be fixed > too. > > Fixes: 1baf9127c482 ("drm/mgag200: Replace simple-KMS with regular atomic helpers") > Cc: <stable@vger.kernel.org> #v6.1+ > Reported-by: Roger Sewell <roger.sewell@cantab.net> > Suggested-by: Roger Sewell <roger.sewell@cantab.net> > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> > --- > drivers/gpu/drm/mgag200/mgag200_drv.h | 4 ++++ > 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 | 26 ++++++++++++++---------- > 5 files changed, 25 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h > index 57c7edcab602..ed90a92b5fcd 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.h > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h > @@ -392,6 +392,10 @@ void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, > .destroy = drm_plane_cleanup, \ > DRM_GEM_SHADOW_PLANE_FUNCS > > +void mgag200_crtc_set_gamma(struct mga_device *mdev, > + const struct drm_format_info *format, > + struct drm_property_blob *gamma_lut); > + > enum drm_mode_status mgag200_crtc_helper_mode_valid(struct drm_crtc *crtc, > const struct drm_display_mode *mode); > int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *new_state); > diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c > index bce267e0f7de..38815cb94c61 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_g200er.c > +++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c > @@ -202,6 +202,8 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc, > > mgag200_g200er_reset_tagfifo(mdev); > > + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); > + > mgag200_enable_display(mdev); > > if (funcs->enable_vidrst) > diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c > index ac957f42abe1..e698a3a499bf 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c > +++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c > @@ -203,6 +203,8 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc, > > mgag200_g200ev_set_hiprilvl(mdev); > > + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); > + > mgag200_enable_display(mdev); > > if (funcs->enable_vidrst) > diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c > index bd6e573c9a1a..7e4ea0046a6b 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_g200se.c > +++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c > @@ -334,6 +334,8 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc, > > mgag200_g200se_set_hiprilvl(mdev, adjusted_mode, format); > > + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); > + > mgag200_enable_display(mdev); > > if (funcs->enable_vidrst) > diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c > index af3ce5a6a636..d2a04b317232 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_mode.c > +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c > @@ -65,9 +65,9 @@ static void mgag200_crtc_set_gamma_linear(struct mga_device *mdev, > } > } > > -static void mgag200_crtc_set_gamma(struct mga_device *mdev, > - const struct drm_format_info *format, > - struct drm_color_lut *lut) > +static void mgag200_crtc_set_gamma_table(struct mga_device *mdev, > + const struct drm_format_info *format, > + struct drm_color_lut *lut) > { > int i; > > @@ -103,6 +103,16 @@ static void mgag200_crtc_set_gamma(struct mga_device *mdev, > } > } > > +void mgag200_crtc_set_gamma(struct mga_device *mdev, > + const struct drm_format_info *format, > + struct drm_property_blob *gamma_lut) > +{ > + if (gamma_lut) > + mgag200_crtc_set_gamma_table(mdev, format, gamma_lut->data); > + else > + mgag200_crtc_set_gamma_linear(mdev, format); > +} Please keep this open-coded its callers. With that changed Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > + > static inline void mga_wait_vsync(struct mga_device *mdev) > { > unsigned long timeout = jiffies + HZ/10; > @@ -616,10 +626,7 @@ void mgag200_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_s > if (crtc_state->enable && crtc_state->color_mgmt_changed) { > const struct drm_format_info *format = mgag200_crtc_state->format; > > - if (crtc_state->gamma_lut) > - mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data); > - else > - mgag200_crtc_set_gamma_linear(mdev, format); > + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); > } > } > > @@ -642,10 +649,7 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_ > if (funcs->pixpllc_atomic_update) > funcs->pixpllc_atomic_update(crtc, old_state); > > - if (crtc_state->gamma_lut) > - mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data); > - else > - mgag200_crtc_set_gamma_linear(mdev, format); > + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); > > mgag200_enable_display(mdev); > > > base-commit: 6c9dbee84cd005bed5f9d07b3a2797ae6414b435
On 18/12/2023 12:31, Thomas Zimmermann wrote: > Hi > > Am 14.12.23 um 17:38 schrieb Jocelyn Falempe: >> When mgag200 switched from simple KMS to regular atomic helpers, >> the initialization of the gamma settings was lost. >> This leads to a black screen, if the bios/uefi doesn't use the same >> pixel color depth. >> This has been fixed with commit ad81e23426a6 ("drm/mgag200: Fix gamma >> lut not initialized.") for most G200, but G200ER, G200EV, G200SE use >> their own version of crtc_helper_atomic_enable() and need to be fixed >> too. >> >> Fixes: 1baf9127c482 ("drm/mgag200: Replace simple-KMS with regular >> atomic helpers") >> Cc: <stable@vger.kernel.org> #v6.1+ >> Reported-by: Roger Sewell <roger.sewell@cantab.net> >> Suggested-by: Roger Sewell <roger.sewell@cantab.net> >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >> --- >> drivers/gpu/drm/mgag200/mgag200_drv.h | 4 ++++ >> 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 | 26 ++++++++++++++---------- >> 5 files changed, 25 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h >> b/drivers/gpu/drm/mgag200/mgag200_drv.h >> index 57c7edcab602..ed90a92b5fcd 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h >> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h >> @@ -392,6 +392,10 @@ void >> mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, >> .destroy = drm_plane_cleanup, \ >> DRM_GEM_SHADOW_PLANE_FUNCS >> +void mgag200_crtc_set_gamma(struct mga_device *mdev, >> + const struct drm_format_info *format, >> + struct drm_property_blob *gamma_lut); >> + >> enum drm_mode_status mgag200_crtc_helper_mode_valid(struct drm_crtc >> *crtc, >> const struct drm_display_mode *mode); >> int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct >> drm_atomic_state *new_state); >> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c >> b/drivers/gpu/drm/mgag200/mgag200_g200er.c >> index bce267e0f7de..38815cb94c61 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_g200er.c >> +++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c >> @@ -202,6 +202,8 @@ static void >> mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc, >> mgag200_g200er_reset_tagfifo(mdev); >> + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); >> + >> mgag200_enable_display(mdev); >> if (funcs->enable_vidrst) >> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c >> b/drivers/gpu/drm/mgag200/mgag200_g200ev.c >> index ac957f42abe1..e698a3a499bf 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c >> +++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c >> @@ -203,6 +203,8 @@ static void >> mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc, >> mgag200_g200ev_set_hiprilvl(mdev); >> + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); >> + >> mgag200_enable_display(mdev); >> if (funcs->enable_vidrst) >> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c >> b/drivers/gpu/drm/mgag200/mgag200_g200se.c >> index bd6e573c9a1a..7e4ea0046a6b 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_g200se.c >> +++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c >> @@ -334,6 +334,8 @@ static void >> mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc, >> mgag200_g200se_set_hiprilvl(mdev, adjusted_mode, format); >> + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); >> + >> mgag200_enable_display(mdev); >> if (funcs->enable_vidrst) >> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c >> b/drivers/gpu/drm/mgag200/mgag200_mode.c >> index af3ce5a6a636..d2a04b317232 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c >> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c >> @@ -65,9 +65,9 @@ static void mgag200_crtc_set_gamma_linear(struct >> mga_device *mdev, >> } >> } >> -static void mgag200_crtc_set_gamma(struct mga_device *mdev, >> - const struct drm_format_info *format, >> - struct drm_color_lut *lut) >> +static void mgag200_crtc_set_gamma_table(struct mga_device *mdev, >> + const struct drm_format_info *format, >> + struct drm_color_lut *lut) >> { >> int i; >> @@ -103,6 +103,16 @@ static void mgag200_crtc_set_gamma(struct >> mga_device *mdev, >> } >> } >> +void mgag200_crtc_set_gamma(struct mga_device *mdev, >> + const struct drm_format_info *format, >> + struct drm_property_blob *gamma_lut) >> +{ >> + if (gamma_lut) >> + mgag200_crtc_set_gamma_table(mdev, format, gamma_lut->data); >> + else >> + mgag200_crtc_set_gamma_linear(mdev, format); >> +} > > Please keep this open-coded its callers. With that changed > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> Thanks for the review, yes I will change that. If no other comments, I will push it to drm-misc-fixes tomorrow.
I just merged it to drm-misc-fixes: https://cgit.freedesktop.org/drm/drm-misc/commit/?h=drm-misc-fixes&id=11f9eb899ecc8c02b769cf8d2532ba12786a7af7 Thanks,
Hi Am 20.12.23 um 14:06 schrieb Jocelyn Falempe: > I just merged it to drm-misc-fixes: > > https://cgit.freedesktop.org/drm/drm-misc/commit/?h=drm-misc-fixes&id=11f9eb899ecc8c02b769cf8d2532ba12786a7af7 > > Thanks, Thanks a lot for the fix. Best regards Thomas >
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 57c7edcab602..ed90a92b5fcd 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -392,6 +392,10 @@ void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, .destroy = drm_plane_cleanup, \ DRM_GEM_SHADOW_PLANE_FUNCS +void mgag200_crtc_set_gamma(struct mga_device *mdev, + const struct drm_format_info *format, + struct drm_property_blob *gamma_lut); + enum drm_mode_status mgag200_crtc_helper_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode); int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *new_state); diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c index bce267e0f7de..38815cb94c61 100644 --- a/drivers/gpu/drm/mgag200/mgag200_g200er.c +++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c @@ -202,6 +202,8 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc, mgag200_g200er_reset_tagfifo(mdev); + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); + mgag200_enable_display(mdev); if (funcs->enable_vidrst) diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c index ac957f42abe1..e698a3a499bf 100644 --- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c +++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c @@ -203,6 +203,8 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc, mgag200_g200ev_set_hiprilvl(mdev); + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); + mgag200_enable_display(mdev); if (funcs->enable_vidrst) diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c index bd6e573c9a1a..7e4ea0046a6b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_g200se.c +++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c @@ -334,6 +334,8 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc, mgag200_g200se_set_hiprilvl(mdev, adjusted_mode, format); + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); + mgag200_enable_display(mdev); if (funcs->enable_vidrst) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index af3ce5a6a636..d2a04b317232 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -65,9 +65,9 @@ static void mgag200_crtc_set_gamma_linear(struct mga_device *mdev, } } -static void mgag200_crtc_set_gamma(struct mga_device *mdev, - const struct drm_format_info *format, - struct drm_color_lut *lut) +static void mgag200_crtc_set_gamma_table(struct mga_device *mdev, + const struct drm_format_info *format, + struct drm_color_lut *lut) { int i; @@ -103,6 +103,16 @@ static void mgag200_crtc_set_gamma(struct mga_device *mdev, } } +void mgag200_crtc_set_gamma(struct mga_device *mdev, + const struct drm_format_info *format, + struct drm_property_blob *gamma_lut) +{ + if (gamma_lut) + mgag200_crtc_set_gamma_table(mdev, format, gamma_lut->data); + else + mgag200_crtc_set_gamma_linear(mdev, format); +} + static inline void mga_wait_vsync(struct mga_device *mdev) { unsigned long timeout = jiffies + HZ/10; @@ -616,10 +626,7 @@ void mgag200_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_s if (crtc_state->enable && crtc_state->color_mgmt_changed) { const struct drm_format_info *format = mgag200_crtc_state->format; - if (crtc_state->gamma_lut) - mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data); - else - mgag200_crtc_set_gamma_linear(mdev, format); + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); } } @@ -642,10 +649,7 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_ if (funcs->pixpllc_atomic_update) funcs->pixpllc_atomic_update(crtc, old_state); - if (crtc_state->gamma_lut) - mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data); - else - mgag200_crtc_set_gamma_linear(mdev, format); + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut); mgag200_enable_display(mdev);
When mgag200 switched from simple KMS to regular atomic helpers, the initialization of the gamma settings was lost. This leads to a black screen, if the bios/uefi doesn't use the same pixel color depth. This has been fixed with commit ad81e23426a6 ("drm/mgag200: Fix gamma lut not initialized.") for most G200, but G200ER, G200EV, G200SE use their own version of crtc_helper_atomic_enable() and need to be fixed too. Fixes: 1baf9127c482 ("drm/mgag200: Replace simple-KMS with regular atomic helpers") Cc: <stable@vger.kernel.org> #v6.1+ Reported-by: Roger Sewell <roger.sewell@cantab.net> Suggested-by: Roger Sewell <roger.sewell@cantab.net> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> --- drivers/gpu/drm/mgag200/mgag200_drv.h | 4 ++++ 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 | 26 ++++++++++++++---------- 5 files changed, 25 insertions(+), 11 deletions(-) base-commit: 6c9dbee84cd005bed5f9d07b3a2797ae6414b435