Re: [PATCH] drm: Exynos: Respect framebuffer pitch for FIMD/Mixer
Hello Javier, 2015-03-27 20:08 GMT+09:00 Javier Martinez Canillas : > Hello Inki, > > On Fri, Mar 27, 2015 at 2:47 AM, Inki Dae wrote: >> >> Right, this is not documented but if you have ever checked exynos drm >> driver tree, then I think you could know how we use the prefix. Of >> course, I don't like to force the use of this prefix but if you and >> other people use prefix in the manner of them, exynos drm tree would be >> a little bit messy. i.e., DRM: EXYNOS, drm: exynos, drm: Exynos, >> drm/exynos, drm/exynos: fimd, drm: exynos: fimd, DRM: EXYNOS: FIMD, ... >> so many cases Do you really want this? >> >> So I will always say "please, use right prefix" Otherwise, I will change >> it while merging as is. >> And I see that other drm drivers have their own way which is not >> documented but just implicitly. >> > > I agree with you that people should follow the convention for subject > lines used in a subsystem and that it is tedious to tell them to fix > the subject and resend but I do agree with Tobias that you should > rethink your mail filters. > > For example I've a filter for all the emails that are directly > addressed to me or that I'm cc'ed to. It's true that you may get some > emails that are not interesting to you just because people cc'ed you > in random patches but I think is better to have false positives than > to have false negatives. Specially since you are a kernel maintainer > and people expect your feedback / ack for patches to get merged. > Added a new filter, which will pick up all emails of CCing me. :) Thanks, Inki Dae >> Thanks, >> Inki Dae >> > > Best regards, > Javier > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm: Exynos: Respect framebuffer pitch for FIMD/Mixer
Hello Inki, On Fri, Mar 27, 2015 at 2:47 AM, Inki Dae wrote: > > Right, this is not documented but if you have ever checked exynos drm > driver tree, then I think you could know how we use the prefix. Of > course, I don't like to force the use of this prefix but if you and > other people use prefix in the manner of them, exynos drm tree would be > a little bit messy. i.e., DRM: EXYNOS, drm: exynos, drm: Exynos, > drm/exynos, drm/exynos: fimd, drm: exynos: fimd, DRM: EXYNOS: FIMD, ... > so many cases Do you really want this? > > So I will always say "please, use right prefix" Otherwise, I will change > it while merging as is. > And I see that other drm drivers have their own way which is not > documented but just implicitly. > I agree with you that people should follow the convention for subject lines used in a subsystem and that it is tedious to tell them to fix the subject and resend but I do agree with Tobias that you should rethink your mail filters. For example I've a filter for all the emails that are directly addressed to me or that I'm cc'ed to. It's true that you may get some emails that are not interesting to you just because people cc'ed you in random patches but I think is better to have false positives than to have false negatives. Specially since you are a kernel maintainer and people expect your feedback / ack for patches to get merged. > Thanks, > Inki Dae > Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm: Exynos: Respect framebuffer pitch for FIMD/Mixer
Hi Daniel, On 2015년 03월 26일 23:48, Daniel Stone wrote: > Hi Inki, > > On Thu, 26 Mar, 2015 at 2:32 PM, Inki Dae wrote: >> Applied. > > Thanks very much. > >> Could you use right prefix of the subject like below when you post patch? >> >> 'drm/exynos: ...', not 'drm: Exynos: ...' >> >> Your email will be filtered from my mailbox if you don't use the right >> prefix so I couldn't check and take care of your patch. > > Ah, I didn't realise this. Maybe it could be good to not filter if the > patch is also directly addressed/CCed to you, rather than a list? > Gustavo Padovan is following this convention and I also will in future, > but I guess it might lead to some patches being dropped from casual > contributors if they don't know this. Sorry for confusing. I don't really want to drop the patches which don't use right prefix - undocumented. I have two filters, one is DRI mailing list, other is drm/exynos. So if you use other prefix, your patch is filtered and stored in my dri-devel mailbox. But as you may know, the mailbox would always be full with many emails which contains various type patches, exynos, omap, sti, radeon, intel and so no. So it wouldn't be easy to pick up only your patch among them. So I'd like to recommend you to use drm/exynos prefix so that I could review and pick it up easily and quickly. Thanks, Inki Dae > > Cheers, > Daniel > >> >> Thanks, >> Inki Dae >> >> On 2015년 03월 24일 17:57, Javier Martinez Canillas wrote: >>> Hello Inki, >>> >>> On Tue, Mar 17, 2015 at 2:24 PM, Daniel Stone >>> wrote: When performing a modeset, use the framebuffer pitch value to set FIMD IMG_SIZE and Mixer SPAN registers. These are both defined as pitch - the distance between contiguous lines (bytes for FIMD, pixels for mixer). Fixes display on Snow (1366x768). Signed-off-by: Daniel Stone Tested-by: Javier Martinez Canillas >>> >>> Any comments on this patch? It would be great to pick this sooner >>> rather than later since it fixes (at least) display output on Snow and >>> HDMI output on Peach Pit/Pi. >>> >>> Best regards, >>> Javier >>> >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm: Exynos: Respect framebuffer pitch for FIMD/Mixer
On 2015년 03월 27일 09:15, Tobias Jakobi wrote: > Inki Dae wrote: >> Hello Javier, >> >> Applied. >> >> Could you use right prefix of the subject like below when you post patch? >> >> 'drm/exynos: ...', not 'drm: Exynos: ...' >> >> Your email will be filtered from my mailbox if you don't use the right >> prefix so I couldn't check and take care of your patch. > I would suggest to change (or drop) the filter settings then. I don't Filters of my mailbox are considered for two cases, one is DRI mailing list, other is drm/exynos. So if you don't use the drm/exynos prefix, then I would try to find your patch in DRI mailing list - I sometimes check the mailing list. However, the mailing list have so much emails so it'd be not easy to pick up only exynos relevant patches among the email lists. As a result, you would get the my review lately. > think you can expect people to jump through these hoops to reach a > maintainer of a kernel subsystem. Especially since this not 'documented' > anywhere. Right, this is not documented but if you have ever checked exynos drm driver tree, then I think you could know how we use the prefix. Of course, I don't like to force the use of this prefix but if you and other people use prefix in the manner of them, exynos drm tree would be a little bit messy. i.e., DRM: EXYNOS, drm: exynos, drm: Exynos, drm/exynos, drm/exynos: fimd, drm: exynos: fimd, DRM: EXYNOS: FIMD, ... so many cases Do you really want this? So I will always say "please, use right prefix" Otherwise, I will change it while merging as is. And I see that other drm drivers have their own way which is not documented but just implicitly. Thanks, Inki Dae > > > > With best wishes, > Tobias > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm: Exynos: Respect framebuffer pitch for FIMD/Mixer
Inki Dae wrote: > Hello Javier, > > Applied. > > Could you use right prefix of the subject like below when you post patch? > > 'drm/exynos: ...', not 'drm: Exynos: ...' > > Your email will be filtered from my mailbox if you don't use the right > prefix so I couldn't check and take care of your patch. I would suggest to change (or drop) the filter settings then. I don't think you can expect people to jump through these hoops to reach a maintainer of a kernel subsystem. Especially since this not 'documented' anywhere. With best wishes, Tobias -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm: Exynos: Respect framebuffer pitch for FIMD/Mixer
Hello Javier, Applied. Could you use right prefix of the subject like below when you post patch? 'drm/exynos: ...', not 'drm: Exynos: ...' Your email will be filtered from my mailbox if you don't use the right prefix so I couldn't check and take care of your patch. Thanks, Inki Dae On 2015년 03월 24일 17:57, Javier Martinez Canillas wrote: > Hello Inki, > > On Tue, Mar 17, 2015 at 2:24 PM, Daniel Stone wrote: >> When performing a modeset, use the framebuffer pitch value to set FIMD >> IMG_SIZE and Mixer SPAN registers. These are both defined as pitch - the >> distance between contiguous lines (bytes for FIMD, pixels for mixer). >> >> Fixes display on Snow (1366x768). >> >> Signed-off-by: Daniel Stone >> Tested-by: Javier Martinez Canillas > > Any comments on this patch? It would be great to pick this sooner > rather than later since it fixes (at least) display output on Snow and > HDMI output on Peach Pit/Pi. > > Best regards, > Javier > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm: Exynos: Respect framebuffer pitch for FIMD/Mixer
Hello Inki, On Tue, Mar 17, 2015 at 2:24 PM, Daniel Stone wrote: > When performing a modeset, use the framebuffer pitch value to set FIMD > IMG_SIZE and Mixer SPAN registers. These are both defined as pitch - the > distance between contiguous lines (bytes for FIMD, pixels for mixer). > > Fixes display on Snow (1366x768). > > Signed-off-by: Daniel Stone > Tested-by: Javier Martinez Canillas Any comments on this patch? It would be great to pick this sooner rather than later since it fixes (at least) display output on Snow and HDMI output on Peach Pit/Pi. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm: Exynos: Respect framebuffer pitch for FIMD/Mixer
When performing a modeset, use the framebuffer pitch value to set FIMD
IMG_SIZE and Mixer SPAN registers. These are both defined as pitch - the
distance between contiguous lines (bytes for FIMD, pixels for mixer).
Fixes display on Snow (1366x768).
Signed-off-by: Daniel Stone
Tested-by: Javier Martinez Canillas
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 8 +---
drivers/gpu/drm/exynos/exynos_mixer.c| 17 ++---
2 files changed, 15 insertions(+), 10 deletions(-)
The VP parts are untested as I don't have Exynos4 hardware, and the
spec is uselessly vague. It does seem to look like the right fix however,
and in the case where the buffer is naturally aligned (pitch == width*bpp),
there should be no change, as bpp==8 for video buffers.
FIMD/GRP and Mixer behaviour have been manually verified on Snow (5250
with eDP/LVDS bridge and HDMI out).
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 925fc69..0d5681f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -147,6 +147,7 @@ struct fimd_win_data {
unsigned intovl_height;
unsigned intfb_width;
unsigned intfb_height;
+ unsigned intfb_pitch;
unsigned intbpp;
unsigned intpixel_format;
dma_addr_t dma_addr;
@@ -537,13 +538,14 @@ static void fimd_win_mode_set(struct exynos_drm_crtc
*crtc,
win_data->offset_y = plane->crtc_y;
win_data->ovl_width = plane->crtc_width;
win_data->ovl_height = plane->crtc_height;
+ win_data->fb_pitch = plane->pitch;
win_data->fb_width = plane->fb_width;
win_data->fb_height = plane->fb_height;
win_data->dma_addr = plane->dma_addr[0] + offset;
win_data->bpp = plane->bpp;
win_data->pixel_format = plane->pixel_format;
- win_data->buf_offsize = (plane->fb_width - plane->crtc_width) *
- (plane->bpp >> 3);
+ win_data->buf_offsize =
+ plane->pitch - (plane->crtc_width * (plane->bpp >> 3));
win_data->line_size = plane->crtc_width * (plane->bpp >> 3);
DRM_DEBUG_KMS("offset_x = %d, offset_y = %d\n",
@@ -709,7 +711,7 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc,
int zpos)
writel(val, ctx->regs + VIDWx_BUF_START(win, 0));
/* buffer end address */
- size = win_data->fb_width * win_data->ovl_height * (win_data->bpp >> 3);
+ size = win_data->fb_pitch * win_data->ovl_height * (win_data->bpp >> 3);
val = (unsigned long)(win_data->dma_addr + size);
writel(val, ctx->regs + VIDWx_BUF_END(win, 0));
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 3518bc4..2e3bc57 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -55,6 +55,7 @@ struct hdmi_win_data {
unsigned intfb_x;
unsigned intfb_y;
unsigned intfb_width;
+ unsigned intfb_pitch;
unsigned intfb_height;
unsigned intsrc_width;
unsigned intsrc_height;
@@ -438,7 +439,7 @@ static void vp_video_buffer(struct mixer_context *ctx, int
win)
} else {
luma_addr[0] = win_data->dma_addr;
chroma_addr[0] = win_data->dma_addr
- + (win_data->fb_width * win_data->fb_height);
+ + (win_data->fb_pitch * win_data->fb_height);
}
if (win_data->scan_flags & DRM_MODE_FLAG_INTERLACE) {
@@ -447,8 +448,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int
win)
luma_addr[1] = luma_addr[0] + 0x40;
chroma_addr[1] = chroma_addr[0] + 0x40;
} else {
- luma_addr[1] = luma_addr[0] + win_data->fb_width;
- chroma_addr[1] = chroma_addr[0] + win_data->fb_width;
+ luma_addr[1] = luma_addr[0] + win_data->fb_pitch;
+ chroma_addr[1] = chroma_addr[0] + win_data->fb_pitch;
}
} else {
ctx->interlace = false;
@@ -469,10 +470,10 @@ static void vp_video_buffer(struct mixer_context *ctx,
int win)
vp_reg_writemask(res, VP_MODE, val, VP_MODE_FMT_MASK);
/* setting size of input image */
- vp_reg_write(res, VP_IMG_SIZE_Y, VP_IMG_HSIZE(win_data->fb_width) |
+ vp_reg_write(res, VP_IMG_SIZE_Y, VP_IMG_HSIZE(win_data->fb_pitch) |
VP_IMG_VSIZE(win_data->fb_height));
/* chroma height has to reduced by 2 to avoid chroma distorions */
- vp_reg_write(res, VP_IMG_SIZE_C, VP_IMG_HSIZE(win_data->fb_width) |
+ vp_reg_write(res, VP_IMG_SIZE_C, VP_IMG_HSIZE(win_data->fb_pitch) |
VP_IMG_VSIZE(win_data
