It's frankly a mess, and the confusion around plane_state->crtc/fb that I fixed up in this series is the least of the problems. Add a todo as a future note of how this could be done a lot better, and with a lot less driver confusion.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- Documentation/gpu/todo.rst | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 2d85f37284a1..63b657ecc9ce 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -72,6 +72,28 @@ Contact: Ville Syrjälä, Daniel Vetter, driver maintainers
Level: Advanced
+Improve plane atomic_check helpers +---------------------------------- + +Aside from the clipped coordinates right above there's a few suboptimal things +with the current helpers: + +- drm_plane_helper_funcs->atomic_check gets called for enabled or disabled + planes. At best this seems to confuse drivers, worst it means they blow up + when the plane is disabled without the CRTC. The only special handling is + resetting values in the plane state structures, which instead should be moved + into the drm_plane_funcs->atomic_duplicate_state functions. + +- Once that's done, helpers could stop calling ->atomic_check for disabled + planes. + +- Then we could go through all the drivers and remove the more-or-less confused + checks for plane_state->fb and plane_state->crtc. + +Contact: Daniel Vetter + +Level: Advanced + Convert early atomic drivers to async commit helpers ----------------------------------------------------
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Liviu Dudau liviu.dudau@arm.com Cc: Brian Starkey brian.starkey@arm.com Cc: --- drivers/gpu/drm/arm/malidp_planes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c index 3c70a53813bf..37715cc6064e 100644 --- a/drivers/gpu/drm/arm/malidp_planes.c +++ b/drivers/gpu/drm/arm/malidp_planes.c @@ -512,7 +512,7 @@ static int malidp_de_plane_check(struct drm_plane *plane, int i, ret; unsigned int block_w, block_h;
- if (!state->crtc || !state->fb) + if (!state->crtc || WARN_ON(!state->fb)) return 0;
fb = state->fb;
On Fri, Dec 13, 2019 at 06:26:04PM +0100, Daniel Vetter wrote:
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Liviu Dudau liviu.dudau@arm.com
Acked-by: Liviu Dudau liviu.dudau@arm.com
Best regards, Liviu
Cc: Brian Starkey brian.starkey@arm.com Cc:
drivers/gpu/drm/arm/malidp_planes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c index 3c70a53813bf..37715cc6064e 100644 --- a/drivers/gpu/drm/arm/malidp_planes.c +++ b/drivers/gpu/drm/arm/malidp_planes.c @@ -512,7 +512,7 @@ static int malidp_de_plane_check(struct drm_plane *plane, int i, ret; unsigned int block_w, block_h;
- if (!state->crtc || !state->fb)
if (!state->crtc || WARN_ON(!state->fb)) return 0;
fb = state->fb;
-- 2.24.0
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Sam Ravnborg sam@ravnborg.org Cc: Boris Brezillon bbrezillon@kernel.org Cc: Nicolas Ferre nicolas.ferre@microchip.com Cc: Alexandre Belloni alexandre.belloni@bootlin.com Cc: Ludovic Desroches ludovic.desroches@microchip.com Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c index 034f202dfe8f..40800ec5700a 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c @@ -604,7 +604,7 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p, int ret; int i;
- if (!state->base.crtc || !fb) + if (!state->base.crtc || WARN_ON(!fb)) return 0;
crtc_state = drm_atomic_get_existing_crtc_state(s->state, s->crtc);
Hi Daniel.
On Fri, Dec 13, 2019 at 06:26:05PM +0100, Daniel Vetter wrote:
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Sam Ravnborg sam@ravnborg.org Cc: Boris Brezillon bbrezillon@kernel.org Cc: Nicolas Ferre nicolas.ferre@microchip.com Cc: Alexandre Belloni alexandre.belloni@bootlin.com Cc: Ludovic Desroches ludovic.desroches@microchip.com Cc: linux-arm-kernel@lists.infradead.org
Applied to drm-misc-next. Looked through the whole series: Acked-by: Sam Ravnborg sam@ravnborg.org
On Fri, Dec 13, 2019 at 08:53:30PM +0100, Sam Ravnborg wrote:
Hi Daniel.
On Fri, Dec 13, 2019 at 06:26:05PM +0100, Daniel Vetter wrote:
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Sam Ravnborg sam@ravnborg.org Cc: Boris Brezillon bbrezillon@kernel.org Cc: Nicolas Ferre nicolas.ferre@microchip.com Cc: Alexandre Belloni alexandre.belloni@bootlin.com Cc: Ludovic Desroches ludovic.desroches@microchip.com Cc: linux-arm-kernel@lists.infradead.org
Applied to drm-misc-next. Looked through the whole series: Acked-by: Sam Ravnborg sam@ravnborg.org
Ok, pulled in the remaining patches, thanks for taking a look. -Daniel
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Shawn Guo shawnguo@kernel.org Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Pengutronix Kernel Team kernel@pengutronix.de Cc: Fabio Estevam festevam@gmail.com Cc: NXP Linux Team linux-imx@nxp.com Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/imx/ipuv3-plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 28826c0aa24a..6776ebb3246d 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -359,7 +359,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane, if (!fb) return 0;
- if (!state->crtc) + if (WARN_ON(!state->crtc)) return -EINVAL;
crtc_state =
On Fri, Dec 13, 2019 at 06:26:06PM +0100, Daniel Vetter wrote:
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Shawn Guo shawnguo@kernel.org Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Pengutronix Kernel Team kernel@pengutronix.de Cc: Fabio Estevam festevam@gmail.com Cc: NXP Linux Team linux-imx@nxp.com Cc: linux-arm-kernel@lists.infradead.org
Oops, subect should be drm/imx: ofc here. -Daniel
drivers/gpu/drm/imx/ipuv3-plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 28826c0aa24a..6776ebb3246d 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -359,7 +359,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane, if (!fb) return 0;
- if (!state->crtc)
if (WARN_ON(!state->crtc)) return -EINVAL;
crtc_state =
-- 2.24.0
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: CK Hu ck.hu@mediatek.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Matthias Brugger matthias.bgg@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-mediatek@lists.infradead.org --- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 540ef2faa40a..f0b0325381e0 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -94,7 +94,7 @@ static int mtk_plane_atomic_check(struct drm_plane *plane, if (!fb) return 0;
- if (!state->crtc) + if (WARN_ON(!state->crtc)) return 0;
ret = mtk_drm_crtc_plane_check(state->crtc, plane,
Hi, Daniel:
On Fri, 2019-12-13 at 18:26 +0100, Daniel Vetter wrote:
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Even though I don't know what is copypasta,
Acked-by: CK Hu ck.hu@mediatek.com
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: CK Hu ck.hu@mediatek.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Matthias Brugger matthias.bgg@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-mediatek@lists.infradead.org
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 540ef2faa40a..f0b0325381e0 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -94,7 +94,7 @@ static int mtk_plane_atomic_check(struct drm_plane *plane, if (!fb) return 0;
- if (!state->crtc)
if (WARN_ON(!state->crtc)) return 0;
ret = mtk_drm_crtc_plane_check(state->crtc, plane,
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Sandy Huang hjc@rock-chips.com Cc: "Heiko Stübner" heiko@sntech.de Cc: linux-arm-kernel@lists.infradead.org Cc: linux-rockchip@lists.infradead.org --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index d04b3492bdac..cecb2cc781f5 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -724,7 +724,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane, int max_scale = win->phy->scl ? FRAC_16_16(8, 1) : DRM_PLANE_HELPER_NO_SCALING;
- if (!crtc || !fb) + if (!crtc || WARN_ON(!fb)) return 0;
crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc);
Am Freitag, 13. Dezember 2019, 18:26:08 CET schrieb Daniel Vetter:
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Sandy Huang hjc@rock-chips.com Cc: "Heiko Stübner" heiko@sntech.de Cc: linux-arm-kernel@lists.infradead.org Cc: linux-rockchip@lists.infradead.org
patch subject should be "drm/rockchip" without the x, otherwise Reviewed-by: Heiko Stuebner heiko@sntech.de
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index d04b3492bdac..cecb2cc781f5 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -724,7 +724,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane, int max_scale = win->phy->scl ? FRAC_16_16(8, 1) : DRM_PLANE_HELPER_NO_SCALING;
- if (!crtc || !fb)
if (!crtc || WARN_ON(!fb)) return 0;
crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc);
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/vc4/vc4_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 4934127f0d76..91e408f7a56e 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -139,7 +139,7 @@ static enum vc4_scaling_mode vc4_get_scaling_mode(u32 src, u32 dst)
static bool plane_enabled(struct drm_plane_state *state) { - return state->fb && state->crtc; + return state->fb && !WARN_ON(!state->crtc); }
static struct drm_plane_state *vc4_plane_duplicate_state(struct drm_plane *plane)
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: David Airlie airlied@linux.ie Cc: Gerd Hoffmann kraxel@redhat.com Cc: virtualization@lists.linux-foundation.org --- drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index bc4bc4475a8c..947e65b51555 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -88,7 +88,7 @@ static int virtio_gpu_plane_atomic_check(struct drm_plane *plane, struct drm_crtc_state *crtc_state; int ret;
- if (!state->fb || !state->crtc) + if (!state->fb || WARN_ON(!state->crtc)) return 0;
crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Rodrigo Siqueira rodrigosiqueiramelo@gmail.com Cc: Haneen Mohammed hamohammed.sa@gmail.com Cc: Daniel Vetter daniel@ffwll.ch --- drivers/gpu/drm/vkms/vkms_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 5fc8f85aaf3d..6d31265a2ab7 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -117,7 +117,7 @@ static int vkms_plane_atomic_check(struct drm_plane *plane, bool can_position = false; int ret;
- if (!state->fb | !state->crtc) + if (!state->fb || WARN_ON(!state->crtc)) return 0;
crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
On 12/13, Daniel Vetter wrote:
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Rodrigo Siqueira rodrigosiqueiramelo@gmail.com Cc: Haneen Mohammed hamohammed.sa@gmail.com Cc: Daniel Vetter daniel@ffwll.ch
drivers/gpu/drm/vkms/vkms_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 5fc8f85aaf3d..6d31265a2ab7 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -117,7 +117,7 @@ static int vkms_plane_atomic_check(struct drm_plane *plane, bool can_position = false; int ret;
- if (!state->fb | !state->crtc)
if (!state->fb || WARN_ON(!state->crtc)) return 0;
crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
-- 2.24.0
Hi,
Sorry, the delay in taking a look at this patch.
I tried to find the whole series for getting the context related to this patch, but I could not find it in my mailbox. Could you shed some light here? Why check fb and crtc is too much? How the WARN_ON fix the issue?
Best Regards
Ps.: in the commit message: "stope" -> "stop"
Hi Rodrigo.
On Tue, Jan 14, 2020 at 06:50:13PM -0500, Rodrigo Siqueira wrote:
On 12/13, Daniel Vetter wrote:
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Rodrigo Siqueira rodrigosiqueiramelo@gmail.com Cc: Haneen Mohammed hamohammed.sa@gmail.com Cc: Daniel Vetter daniel@ffwll.ch
drivers/gpu/drm/vkms/vkms_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 5fc8f85aaf3d..6d31265a2ab7 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -117,7 +117,7 @@ static int vkms_plane_atomic_check(struct drm_plane *plane, bool can_position = false; int ret;
- if (!state->fb | !state->crtc)
if (!state->fb || WARN_ON(!state->crtc)) return 0;
crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
-- 2.24.0
Hi,
Sorry, the delay in taking a look at this patch.
I tried to find the whole series for getting the context related to this patch, but I could not find it in my mailbox. Could you shed some light here? Why check fb and crtc is too much? How the WARN_ON fix the issue?
You can find some background in the first patch: https://lists.freedesktop.org/archives/dri-devel/2019-December/248981.html
Hope this sched enough light on the topic.
Sam
Hi Sam,
On 01/15, Sam Ravnborg wrote:
Hi Rodrigo.
On Tue, Jan 14, 2020 at 06:50:13PM -0500, Rodrigo Siqueira wrote:
On 12/13, Daniel Vetter wrote:
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Rodrigo Siqueira rodrigosiqueiramelo@gmail.com Cc: Haneen Mohammed hamohammed.sa@gmail.com Cc: Daniel Vetter daniel@ffwll.ch
drivers/gpu/drm/vkms/vkms_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 5fc8f85aaf3d..6d31265a2ab7 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -117,7 +117,7 @@ static int vkms_plane_atomic_check(struct drm_plane *plane, bool can_position = false; int ret;
- if (!state->fb | !state->crtc)
if (!state->fb || WARN_ON(!state->crtc)) return 0;
crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
-- 2.24.0
Hi,
Sorry, the delay in taking a look at this patch.
I tried to find the whole series for getting the context related to this patch, but I could not find it in my mailbox. Could you shed some light here? Why check fb and crtc is too much? How the WARN_ON fix the issue?
You can find some background in the first patch: https://lists.freedesktop.org/archives/dri-devel/2019-December/248981.html
Thanks for the link.
Reviewed-by: Rodrigo Siqueira rodrigosiqueira@gmail.com Tested-by: Rodrigo Siqueira rodrigosiqueira@gmail.com
Best Regards
Hope this sched enough light on the topic.
Sam
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Shawn Guo shawnguo@kernel.org --- drivers/gpu/drm/zte/zx_plane.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c index 086c50fac689..c8f7b21fa09e 100644 --- a/drivers/gpu/drm/zte/zx_plane.c +++ b/drivers/gpu/drm/zte/zx_plane.c @@ -54,7 +54,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane, int min_scale = FRAC_16_16(1, 8); int max_scale = FRAC_16_16(8, 1);
- if (!crtc || !fb) + if (!crtc || WARN_ON(!fb)) return 0;
crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state, @@ -281,7 +281,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane, struct drm_crtc *crtc = plane_state->crtc; struct drm_crtc_state *crtc_state;
- if (!crtc || !fb) + if (!crtc || WARN_ON(!fb)) return 0;
crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state,
dri-devel@lists.freedesktop.org
-
CK Hu -
Daniel Vetter -
Daniel Vetter -
Heiko Stübner -
Liviu Dudau -
Rodrigo Siqueira -
Sam Ravnborg