From: "Noralf Trønnes" <noralf@tronnes.org> To: noralf@tronnes.org Cc: dave.stevenson@raspberrypi.com, david@lechnology.com, devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, maxime@cerno.tech, robh+dt@kernel.org, sam@ravnborg.org, thierry.reding@gmail.com Subject: Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver Date: Sun, 20 Feb 2022 19:11:14 +0100 [thread overview] Message-ID: <388f7dc3-afcd-e4c3-592f-8e8401819371@tronnes.org> (raw) In-Reply-To: <35cd42a3-5183-2f21-8728-ee9d65dd1740@tronnes.org> > Den 20.02.2022 11.04, skrev Sam Ravnborg: > > Hi Noralf, > > > >>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) > >>> +{ > >>> + struct device *dev = dbidev->drm.dev; > >>> + u32 width_mm = 0, height_mm = 0; > >>> + struct display_timing timing; > >>> + struct videomode vm; > >>> + int ret; > >>> + > >>> + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); > >>> + if (ret) { > >>> + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); > >>> + return ret; > >>> + } > >>> + > >>> + videomode_from_timing(&timing, &vm); > >>> + > >>> + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || > >>> + (vm.hback_porch + vm.hactive) > 0xffff || > >>> + !vm.vactive || vm.vfront_porch || vm.vsync_len || > >>> + (vm.vback_porch + vm.vactive) > 0xffff || > >>> + vm.flags) { > >>> + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > >>> + return -EINVAL; > >>> + } > >> We should have a helper that implements this. Maybe the display_timing > >> => display_mode helper could do it. > > > > It would be nice with a drm_display_timing_to_mode() but that can come > > later - the comment above should not be understood that I consider it > > mandatory for this driver. > > > > I did consider adding an of_get_drm_panel_mode() fashioned after > of_get_drm_display_mode() but I didn't find any other driver that would > actually be able to use it and I would have to do some substraction to > get back the {h,v}front_porch values that I need and the optional pixel > clock calculation becomes more complex acting from a drm_display_mode so > I decided against it. > > Looking at it now, what I could do is add a function like what > of_get_videomode() does for "display-timings": > > /** > * of_get_panel_videomode - get the panel-timing videomode from devicetree > * @np: devicenode containing the panel-timing subnode > * @vm: returns the videomode > * > * Returns: > * Zero on success, negative error code on failure. > **/ > int of_get_panel_videomode(struct device_node *np, struct videomode *vm) > { > struct display_timing timing; > int ret; > > ret = of_get_display_timing(np, "panel-timing", &timing); > if (ret) > return ret; > > videomode_from_timing(&timing, vm); > > return 0; > } > > This could also be used by panel-lvds and 2 fbdev drivers, the other > panel-timing users need/use the display_timing itself, some for bounds > checking. Scratch that, since videomode is to be avoided I tried adding a drm_display_mode function and it didn't complicate matter as I though it would so I'll do that instead: static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) { struct device *dev = dbidev->drm.dev; u32 width_mm = 0, height_mm = 0; u16 hback_porch, vback_porch; struct videomode vm; int ret; ret = of_get_drm_panel_display_mode(dev->of_node, mode, NULL); if (ret) { dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); return ret; } mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; hback_porch = mode->htotal - mode->hsync_end; vback_porch = mode->vtotal - mode->vsync_end; if (mode->hsync_end > mode->hdisplay || (hback_porch + mode->hdisplay) > 0xffff || mode->vsync_end > mode->vdisplay || (vback_porch + mode->vdisplay) > 0xffff || mode->flags) { dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); return -EINVAL; } /* The driver doesn't use the pixel clock but it is mandatory so fake one if not set */ if (!mode->pixelclock) mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; dbidev->top_offset = vback_porch; dbidev->left_offset = hback_porch; return 0; } int of_get_drm_panel_display_mode(struct device_node *np, struct drm_display_mode *dmode, u32 *bus_flags) { u32 width_mm = 0, height_mm = 0; struct display_timing timing; struct videomode vm; int ret; ret = of_get_display_timing(np, "panel-timing", &timing); if (ret) return ret; videomode_from_timing(&timing, vm); memset(dmode, 0, sizeof(*dmode)); drm_display_mode_from_videomode(&vm, dmode); if (bus_flags) drm_bus_flags_from_videomode(&vm, bus_flags); ret = of_property_read_u32(np, "width-mm", &width_mm); if (ret && ret != -EINVAL) return ret; ret = of_property_read_u32(np, "height-mm", &height_mm); if (ret && ret != -EINVAL) return ret; mode->width_mm = width_mm; mode->height_mm = height_mm; drm_mode_debug_printmodeline(dmode); return 0; } EXPORT_SYMBOL_GPL(of_get_drm_display_mode); Noralf.
WARNING: multiple messages have this Message-ID (diff)
From: "Noralf Trønnes" <noralf@tronnes.org> To: noralf@tronnes.org Cc: devicetree@vger.kernel.org, david@lechnology.com, dave.stevenson@raspberrypi.com, dri-devel@lists.freedesktop.org, robh+dt@kernel.org, thierry.reding@gmail.com, maxime@cerno.tech, sam@ravnborg.org Subject: Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver Date: Sun, 20 Feb 2022 19:11:14 +0100 [thread overview] Message-ID: <388f7dc3-afcd-e4c3-592f-8e8401819371@tronnes.org> (raw) In-Reply-To: <35cd42a3-5183-2f21-8728-ee9d65dd1740@tronnes.org> > Den 20.02.2022 11.04, skrev Sam Ravnborg: > > Hi Noralf, > > > >>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) > >>> +{ > >>> + struct device *dev = dbidev->drm.dev; > >>> + u32 width_mm = 0, height_mm = 0; > >>> + struct display_timing timing; > >>> + struct videomode vm; > >>> + int ret; > >>> + > >>> + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); > >>> + if (ret) { > >>> + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); > >>> + return ret; > >>> + } > >>> + > >>> + videomode_from_timing(&timing, &vm); > >>> + > >>> + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || > >>> + (vm.hback_porch + vm.hactive) > 0xffff || > >>> + !vm.vactive || vm.vfront_porch || vm.vsync_len || > >>> + (vm.vback_porch + vm.vactive) > 0xffff || > >>> + vm.flags) { > >>> + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > >>> + return -EINVAL; > >>> + } > >> We should have a helper that implements this. Maybe the display_timing > >> => display_mode helper could do it. > > > > It would be nice with a drm_display_timing_to_mode() but that can come > > later - the comment above should not be understood that I consider it > > mandatory for this driver. > > > > I did consider adding an of_get_drm_panel_mode() fashioned after > of_get_drm_display_mode() but I didn't find any other driver that would > actually be able to use it and I would have to do some substraction to > get back the {h,v}front_porch values that I need and the optional pixel > clock calculation becomes more complex acting from a drm_display_mode so > I decided against it. > > Looking at it now, what I could do is add a function like what > of_get_videomode() does for "display-timings": > > /** > * of_get_panel_videomode - get the panel-timing videomode from devicetree > * @np: devicenode containing the panel-timing subnode > * @vm: returns the videomode > * > * Returns: > * Zero on success, negative error code on failure. > **/ > int of_get_panel_videomode(struct device_node *np, struct videomode *vm) > { > struct display_timing timing; > int ret; > > ret = of_get_display_timing(np, "panel-timing", &timing); > if (ret) > return ret; > > videomode_from_timing(&timing, vm); > > return 0; > } > > This could also be used by panel-lvds and 2 fbdev drivers, the other > panel-timing users need/use the display_timing itself, some for bounds > checking. Scratch that, since videomode is to be avoided I tried adding a drm_display_mode function and it didn't complicate matter as I though it would so I'll do that instead: static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) { struct device *dev = dbidev->drm.dev; u32 width_mm = 0, height_mm = 0; u16 hback_porch, vback_porch; struct videomode vm; int ret; ret = of_get_drm_panel_display_mode(dev->of_node, mode, NULL); if (ret) { dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); return ret; } mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; hback_porch = mode->htotal - mode->hsync_end; vback_porch = mode->vtotal - mode->vsync_end; if (mode->hsync_end > mode->hdisplay || (hback_porch + mode->hdisplay) > 0xffff || mode->vsync_end > mode->vdisplay || (vback_porch + mode->vdisplay) > 0xffff || mode->flags) { dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); return -EINVAL; } /* The driver doesn't use the pixel clock but it is mandatory so fake one if not set */ if (!mode->pixelclock) mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; dbidev->top_offset = vback_porch; dbidev->left_offset = hback_porch; return 0; } int of_get_drm_panel_display_mode(struct device_node *np, struct drm_display_mode *dmode, u32 *bus_flags) { u32 width_mm = 0, height_mm = 0; struct display_timing timing; struct videomode vm; int ret; ret = of_get_display_timing(np, "panel-timing", &timing); if (ret) return ret; videomode_from_timing(&timing, vm); memset(dmode, 0, sizeof(*dmode)); drm_display_mode_from_videomode(&vm, dmode); if (bus_flags) drm_bus_flags_from_videomode(&vm, bus_flags); ret = of_property_read_u32(np, "width-mm", &width_mm); if (ret && ret != -EINVAL) return ret; ret = of_property_read_u32(np, "height-mm", &height_mm); if (ret && ret != -EINVAL) return ret; mode->width_mm = width_mm; mode->height_mm = height_mm; drm_mode_debug_printmodeline(dmode); return 0; } EXPORT_SYMBOL_GPL(of_get_drm_display_mode); Noralf.
next prev parent reply other threads:[~2022-02-20 18:11 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-02-18 15:11 [PATCH v4 0/3] drm/tiny: Add MIPI DBI compatible SPI driver Noralf Trønnes 2022-02-18 15:11 ` Noralf Trønnes 2022-02-18 15:11 ` [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels Noralf Trønnes 2022-02-18 15:11 ` Noralf Trønnes 2022-02-19 15:25 ` Sam Ravnborg 2022-02-19 15:25 ` Sam Ravnborg 2022-02-21 2:36 ` Rob Herring 2022-02-21 2:36 ` Rob Herring 2022-02-21 11:31 ` Noralf Trønnes 2022-02-21 11:31 ` Noralf Trønnes 2022-02-22 17:26 ` Rob Herring 2022-02-22 17:26 ` Rob Herring 2022-02-18 15:11 ` [PATCH v4 2/3] drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev Noralf Trønnes 2022-02-18 15:11 ` Noralf Trønnes 2022-02-19 15:25 ` Sam Ravnborg 2022-02-19 15:25 ` Sam Ravnborg 2022-02-18 15:11 ` [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver Noralf Trønnes 2022-02-18 15:11 ` Noralf Trønnes 2022-02-19 22:10 ` Sam Ravnborg 2022-02-19 22:10 ` Sam Ravnborg 2022-02-20 10:04 ` Sam Ravnborg 2022-02-20 14:19 ` Noralf Trønnes 2022-02-20 18:11 ` Noralf Trønnes [this message] 2022-02-20 18:11 ` Noralf Trønnes 2022-02-20 19:57 ` Sam Ravnborg 2022-02-20 19:57 ` Sam Ravnborg 2022-02-20 20:34 ` Noralf Trønnes 2022-02-20 20:34 ` Noralf Trønnes 2022-02-20 21:30 ` Sam Ravnborg 2022-02-20 21:30 ` Sam Ravnborg 2022-02-20 15:59 ` Noralf Trønnes 2022-02-20 15:59 ` Noralf Trønnes 2022-02-20 21:32 ` Sam Ravnborg 2022-02-20 21:32 ` Sam Ravnborg 2022-02-21 9:05 ` Maxime Ripard 2022-02-21 9:05 ` Maxime Ripard
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=388f7dc3-afcd-e4c3-592f-8e8401819371@tronnes.org \ --to=noralf@tronnes.org \ --cc=dave.stevenson@raspberrypi.com \ --cc=david@lechnology.com \ --cc=devicetree@vger.kernel.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=maxime@cerno.tech \ --cc=robh+dt@kernel.org \ --cc=sam@ravnborg.org \ --cc=thierry.reding@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.