acassis commented on code in PR #17294:
URL: https://github.com/apache/nuttx/pull/17294#discussion_r2507079818


##########
arch/arm64/src/bcm2711/bcm2711_fb.c:
##########
@@ -0,0 +1,471 @@
+/****************************************************************************
+ * arch/arm64/src/bcm2711/bcm2711_fb.c
+ *
+ * Contributed by Matteo Golin
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <debug.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <errno.h>
+
+#include <arch/board/board.h>
+#include <nuttx/arch.h>
+#include <nuttx/mutex.h>
+#include <nuttx/semaphore.h>
+#include <nuttx/video/fb.h>
+
+#include "bcm2711_mailbox.h"
+#include "arm64_arch.h"
+#include "arm64_gic.h"
+#include "chip.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Screen resolution */
+
+#define FB_WIDTH (1920)
+#define FB_HEIGHT (1080)
+
+/* Bits per pixel (32 for RGB) */
+
+#define FB_BPP (32)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct bcm2711_fb_s
+{
+  struct fb_vtable_s vtable; /* vtable device */
+  void *fb;                  /* Frame buffer pointer */
+  uint32_t fbsize;           /* Size of frame buffer in bytes */
+  int dispno;                /* Display number */
+  bool inited;               /* True when initialized */
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int bcm2711_getvideoinfo(struct fb_vtable_s *vtable,
+                                struct fb_videoinfo_s *vinfo);
+static int bcm2711_getplaneinfo(struct fb_vtable_s *vtable, int planeno,
+                                struct fb_planeinfo_s *pinfo);
+
+/* TODO: implement functions for other features available with the RPi
+ * frame buffer. There is some mailbox commands for a cursor, for instance.
+ */
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* NOTE: It appears that the frame buffer can refer to either HDMI0 or HDMI1,
+ * depending which one is plugged into the display. I don't know what happens
+ * when both are plugged in at once, since I only have a single display to
+ * test with at the moment. I will therefore just refer to it generally as
+ * display 0, but some way to differentiate later would be beneficial.
+ */
+
+static struct bcm2711_fb_s g_bcm2711_fb0 =
+{
+  .inited = 0,
+  .dispno = 0,
+  .vtable =
+    {
+      .getvideoinfo    = bcm2711_getvideoinfo,
+      .getplaneinfo    = bcm2711_getplaneinfo,
+    }
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: bcm2711_getvideoinfo
+ *
+ * Description:
+ *   Get the videoinfo for the framebuffer. (ioctl Entrypoint:
+ *   FBIOGET_VIDEOINFO)
+ *
+ * Input Parameters:
+ *   vtable - Framebuffer driver object
+ *   vinfo  - Returned videoinfo object
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+static int bcm2711_getvideoinfo(struct fb_vtable_s *vtable,
+                                struct fb_videoinfo_s *vinfo)
+{
+  int err;
+  uint32_t bpp;
+  uint32_t xres;
+  uint32_t yres;
+
+  DEBUGASSERT(vtable != NULL);
+  DEBUGASSERT(vinfo != NULL);
+
+  vinfo->nplanes = 1; /* Only one plane supported */
+
+  err = bcm2711_mbox_getdisp(&xres, &yres);
+  if (err < 0)
+    {
+      gerr("Couldn't get display dimensions: %d", err);
+      return err;
+    }
+
+  vinfo->xres = xres;
+  vinfo->yres = yres;
+
+  err = bcm2711_mbox_getdepth(&bpp);
+  if (err < 0)
+    {
+      gerr("Couldn't get display depth: %d", err);
+      return err;
+    }
+
+  /* NOTE: there appears to be no FB_FMT_* descriptors for BGR pixel
+   * ordering, so I will assume RGB always.
+   *
+   * TODO: not even sure how to check for some of the formats. No idea if 16
+   * bpp is the 555 or 565 version.
+   * Should I also be making an attempt to set pixel order to RGB if the
+   * mailbox reports BGR?
+   */
+
+  err = bcm2711_mbox_getalpha(&xres);
+  if (err < 0)
+    {
+      gerr("Couldn't get alpha mode: %d", err);
+      return err;
+    }
+
+  /* TODO: this method ignores the case when the alpha channel is reversed.
+   * Not sure how the frame buffer upper-half can handle that.
+   */
+
+  if (xres == 1)
+    {
+      gwarn("Alpha channel reversed, this is not handled");
+    }
+  else
+    {
+      ginfo("Alpha channel: %s", xres == 0 ? "enabled" : "ignored");
+    }
+
+  switch (bpp)
+    {
+    case 4:
+      DEBUGASSERT(xres == 2);
+      vinfo->fmt = FB_FMT_RGB4;
+      break;
+    case 8:
+      DEBUGASSERT(xres == 2);
+      vinfo->fmt = FB_FMT_RGB8;
+      break;
+    case 16:
+      vinfo->fmt = xres == 0 ? FB_FMT_RGBA16 : FB_FMT_RGB16_555;
+      break;
+    case 24:
+      DEBUGASSERT(xres == 2);
+      vinfo->fmt = FB_FMT_RGB24;
+      break;
+    case 32:
+      vinfo->fmt = xres == 0 ? FB_FMT_RGBA32 : FB_FMT_RGB32;
+      break;
+    default:
+      gerr("Unknown depth of %u bpp", bpp);
+      return -EIO;
+    }
+
+#ifdef CONFIG_FB_OVERLAY
+  vinfo->noverlays = 0; /* No overlays supported */
+#endif
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: pinephone_getplaneinfo
+ *
+ * Description:
+ *   Get the planeinfo for the framebuffer. (ioctl Entrypoint:
+ *   FBIOGET_PLANEINFO)
+ *
+ * Input Parameters:
+ *   vtable - Framebuffer driver object
+ *   pinfo  - Returned planeinfo object
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+static int bcm2711_getplaneinfo(struct fb_vtable_s *vtable, int planeno,
+                                struct fb_planeinfo_s *pinfo)
+{
+  int err;
+  uint32_t result;
+  struct bcm2711_fb_s *priv;
+
+  DEBUGASSERT(vtable != NULL);
+  DEBUGASSERT(pinfo != NULL);
+  DEBUGASSERT(planeno == 0); /* Only one supported plane */
+
+  priv = (struct bcm2711_fb_s *)vtable;

Review Comment:
   suggestion: 
     struct bcm2711_fb_s *priv = (struct bcm2711_fb_s *)vtable;
   
     DEBUGASSERT(priv != NULL);
   
   This way you don't need to debug assert vtable and will keep priv 
declaration/attribution in a single line



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to