On Thu, Nov 22, 2012 at 12:06 PM, 김승우 sw0312.kim@samsung.com wrote:
On 2012년 11월 21일 20:36, Rahul Sharma wrote:
Hi Seung Woo,
Thanks for your inputs. Please find my response below.
On Wed, Nov 21, 2012 at 2:12 PM, 김승우 sw0312.kim@samsung.com wrote:
Hi Rahul,
Control part seems good, and my comment is below.
On 2012년 11월 10일 01:21, Rahul Sharma wrote:
This patch adds code for composing AVI and AUI info frames and send them every VSYNC.
This patch is important for hdmi certification.
Signed-off-by: Fahad Kunnathadi fahad.k@samsung.com Signed-off-by: Shirish S s.shirish@samsung.com Signed-off-by: Rahul Sharma rahul.sharma@samsung.com
drivers/gpu/drm/exynos/exynos_hdmi.c | 97 +++++++++++++++++++++++++++++++++- drivers/gpu/drm/exynos/exynos_hdmi.h | 23 ++++++++ drivers/gpu/drm/exynos/regs-hdmi.h | 17 +++++- 3 files changed, 133 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 2c115f8..bb8a045 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
<snip>
@@ -1993,6 +2084,8 @@ static void hdmi_mode_set(void *ctx, void *mode)
DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
hdata->cur_video_id = drm_match_cea_mode(mode);How do you think about using predefined cea video id in struct hdmi_conf? drm_mode does not have cea video id, so drm_match_cea_mode() compares only mode information. Considering this, IMHO, cea video id can be embedded in struct hdmi_conf.
I feel, It will leads to duplication of video id information. In edid_cea_modes, modes are strictly arranged in the order of respective cea video ID codes. "drm_add_edid_modes" also passes the cea codes (recieved after edid data parsing) as the index to edid_cea_modes to get mode details.
It might be a concern related with your first patch, anyway edid_cea_modes has few pair of exact same modes because struct drm_mode does not have picture ratio. For example, video id 2 and 3 have exact same values for struct drm_mode. So cea video id can be used to get a mode, but a drm_mode is not sufficient to get exact video id. Considering that exynos hdmi does not support video ids with same mode, I suggested video id in struct hdmi_conf. At the point of exynos drm, I can ack this patch.
You are right. This ambiguity is still present about the video code when drm framework sets the mode to hdmi. hdmi_check_timing also doesn't care about picture aspect ratio. I am not sure how to get exact vic from the mode.
I have submitted another patch that where vic is provided the hdmi_conf. I preferred 16:9 aspect ratio. Kindly review that.
regards, Rahul Sharma
Secondly, mode to cea code translation is required by all platforms for AVI packet composition. By adding it to hdmi_conf, we are limiting its usage for exynos.
I agree with you at this point. I quickly checked i915 and radeon and I found that they use fixed value for avi packet at sw level, but I don't have information hw can properly build avi packet. If they also need video id for building avi packet, video id translation can be used.
Best Regards,
- Seung-Woo Kim
regards, Rahul Sharma.
conf_idx = hdmi_conf_index(hdata, mode); if (conf_idx >= 0) hdata->cur_conf = conf_idx;<snip>
Thanks and Regards,
- Seung-Woo Kim
-- Seung-Woo Kim Samsung Software R&D Center --
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel