Rockchip Handhelds: RGA scaling doesn't honor aspect ratio on non-rotated displays

[asoderq]

Description

RGA scaling in drm_go2 driver is broken for devices with non-rotated screens. This is already stated in the code: https://github.com/libretro/RetroArch/blob/83655420f6b1e43f3e430a90e8dc1b9015c7c961/gfx/drivers_context/drm_go2_ctx.c#L126 But I have also verified it on several Rockchip based devices with non-rotated screens.

There are plenty of Rockchip handhelds with non-rotated screen. So it would be nice to get this working for all of them! :)

Expected behavior

RGA scaling should honor configured aspect ratio, it does so on devices with rotated screens but on devices with non-rotated screens it scales to the native resolution.

Actual behavior

For example on Anbernic RG503 which uses the PS VITA OLED screen, 960x544 non-rotated, everything is scaled to native resolution. On my Odroid Go Advance, 320x480 rotated, the aspect ratio is honored.

For both cases settings->video->scaling->aspect_ratio->core provided is used.

Steps to reproduce the bug

Just try enable RGA scaling on devices with non-rotated screens.

What I have tested so far

I naively changed a few lines which apparently seemed wrong, but when doing so I get an error that RgaBlit failed and I get no display output at all, but only hear sound.

This is what I did:

diff --git a/gfx/drivers_context/drm_go2_ctx.c b/gfx/drivers_context/drm_go2_ctx.c
index b758137703..93bcd12fdf 100644
--- a/gfx/drivers_context/drm_go2_ctx.c
+++ b/gfx/drivers_context/drm_go2_ctx.c
@@ -120,18 +120,17 @@ static void *gfx_ctx_go2_drm_init(void *video_driver)
 
    drm->display       = go2_display_create();
 
-   drm->native_width  = go2_display_height_get(drm->display);
-   drm->native_height = go2_display_width_get(drm->display);
+   drm->native_width  = go2_display_width_get(drm->display);
+   drm->native_height = go2_display_height_get(drm->display);
 
    /* This driver should only be used on rotated screens */
-   if (drm->native_width < drm->native_height)
-   {
+//   if (drm->native_width < drm->native_height)
+//   {
       /* This should be fixed by using wayland/weston... */
-      go2_display_destroy(drm->display);
-      free(drm);
-      return NULL;
-   }
-
+//      go2_display_destroy(drm->display);
+//      free(drm);
+//      return NULL;
+//   }
    drm->presenter     = go2_presenter_create(drm->display,
          DRM_FORMAT_RGB565, 0xff000000, true);
 
@@ -337,7 +336,7 @@ static void gfx_ctx_go2_drm_swap_buffers(void *data)
          surface,
          src_x, src_y, src_w, src_h,
          out_y, out_x, out_h, out_w,
-         GO2_ROTATION_DEGREES_270, 2);
+         GO2_ROTATION_DEGREES_0, 2);
    go2_context_surface_unlock(drm->context, surface);
 #endif
 }

@davidgfnet added the comment in code about this driver only being for rotated displays. Do you have additional info on why that is and what could be done to generalize the driver to support all screens?

I would appreciate any ideas on what needs to be done!

[davidgfnet]

Hey there! As you correctly identified this is a driver that uses RGA to rotate the image. This is used in all the OGA clones that have screens that are actually taller than wider (just physically rotated 90 degrees). For the non rotated screens (we had at least one model that had one of those, rg351v iirc) this driver essentially fails to load (hence the check that you mention). The idea is that the driver is not used in favor of the regular Drm driver (that does not support rotation). So if your screen is akready "ok" in the sense that it is natively oriented, you should ignore this driver. Check out the rg351v configs, you might need to mimic what they do. Thanks!

[asoderq]

Sorry for the late reply, was traveling for 2 weeks.

So if your screen is akready "ok" in the sense that it is natively oriented, you should ignore this driver.

AFAIK go2 driver also uses RGA to scale, which is exactly what I want. So I don't think the driver should be ignored, but rather fixed to also support non-rotated screens. So my question is about how that can be done...

[asoderq]

I got it working now on my rg503: It is going to be tested on other devices with non-rotated screens such as rg351v/mp and rg353p thanks to @fewtarius

Below are the changes I made:

diff --git a/gfx/drivers_context/drm_go2_ctx.c b/gfx/drivers_context/drm_go2_ctx.c
index b758137703..a6de933027 100644
--- a/gfx/drivers_context/drm_go2_ctx.c
+++ b/gfx/drivers_context/drm_go2_ctx.c
@@ -120,20 +120,11 @@ static void *gfx_ctx_go2_drm_init(void *video_driver)
 
    drm->display       = go2_display_create();
 
-   drm->native_width  = go2_display_height_get(drm->display);
-   drm->native_height = go2_display_width_get(drm->display);
-
-   /* This driver should only be used on rotated screens */
-   if (drm->native_width < drm->native_height)
-   {
-      /* This should be fixed by using wayland/weston... */
-      go2_display_destroy(drm->display);
-      free(drm);
-      return NULL;
-   }
+   drm->native_width  = go2_display_width_get(drm->display);
+   drm->native_height = go2_display_height_get(drm->display);
 
    drm->presenter     = go2_presenter_create(drm->display,
-         DRM_FORMAT_RGB565, 0xff000000, true);
+         DRM_FORMAT_XRGB8888, 0xff000000, true);
 
    return drm;
 }
@@ -336,8 +327,8 @@ static void gfx_ctx_go2_drm_swap_buffers(void *data)
    go2_presenter_post(drm->presenter,
          surface,
          src_x, src_y, src_w, src_h,
-         out_y, out_x, out_h, out_w,
-         GO2_ROTATION_DEGREES_270, 2);
+         out_x, out_y, out_w, out_h,
+         GO2_ROTATION_DEGREES_0, 2);
    go2_context_surface_unlock(drm->context, surface);
 #endif
 }

Though it needs to be generalized so that both rotated and non-rotated screens work.

[fewtarius]

RG353P, RG351V, and RG351MP all tested fine.

[asoderq]

I have decided to not continue work on generalizing this. I hope we will get a mainline kernel running on this devices and I may then revisit hardware scaling support in RA. Atm not willing to invest time in interfaces that may change. Anyway people that wants RGA scaling on their non-rotated rockchip devices (with bsp kernel) may patch RA with above posted diff.