On Friday 01 of November 2013 16:01:23 Sean Paul wrote:
> On Thu, Oct 31, 2013 at 8:19 PM, Tomasz Figa <tomasz.figa@xxxxxxxxx>
wrote:
> > Hi Sean,
> >
> > On Tuesday 29 of October 2013 12:12:52 Sean Paul wrote:
[snip]
> >> -static void fimd_win_mode_set(struct device *dev,
> >> - struct exynos_drm_overlay *overlay)
> >> +static void fimd_win_mode_set(struct exynos_drm_manager *mgr,
> >> + struct exynos_drm_overlay *overlay)
> >>
> >> {
> >>
> >> - struct fimd_context *ctx = get_fimd_context(dev);
> >> + struct fimd_context *ctx = mgr->ctx;
> >>
> >> struct fimd_win_data *win_data;
> >> int win;
> >> unsigned long offset;
> >>
> >> if (!overlay) {
> >>
> >> - dev_err(dev, "overlay is NULL\n");
> >> + DRM_ERROR("overlay is NULL\n");
> >
> > This change does not seem to be related to $subject.
>
> It is. fimd_win_mode_set does not take dev as an argument any longer,
> as such it's undefined.
Right, I have overlooked this.
> >> return;
> >>
> >> }
> >>
> >> @@ -231,9 +233,8 @@ static void fimd_win_mode_set(struct device *dev,
> >>
> >> overlay->fb_width, overlay->crtc_width);
> >>
> >> }
> >>
> >> -static void fimd_win_set_pixfmt(struct device *dev, unsigned int
> >> win)
> >> +static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned
> >> int
> >> win) {
> >
> > Again not really related to $subject. Maybe this should be done in a
> > preparatory patch preceeding this one? (+ same comment for several
> > identical changes below)
>
> I think it's directly related to the subject. We no longer pass dev as
> an argument, so that has indirect effects on other functions.
Fine.
> >> - struct fimd_context *ctx = get_fimd_context(dev);
> >>
> >> struct fimd_win_data *win_data = &ctx->win_data[win];
> >> unsigned long val;
> >
> > [snip]
> >
> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
> >> b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c index aebcc0e..ca0a87f
> >> 100644
> >> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
> >> @@ -129,11 +129,9 @@ static struct edid *drm_hdmi_get_edid(struct
> >> device *dev,
> >>
> >> return NULL;
> >>
> >> }
> >>
> >> -
> >> -static int drm_hdmi_check_mode(struct device *dev,
> >> +static int drm_hdmi_check_mode_ctx(struct drm_hdmi_context *ctx,
> >>
> >> struct drm_display_mode *mode)
> >>
> >> {
> >>
> >> - struct drm_hdmi_context *ctx = to_context(dev);
> >>
> >> int ret = 0;
> >>
> >> /*
> >>
> >> @@ -153,6 +151,14 @@ static int drm_hdmi_check_mode(struct device
> >> *dev,
> >>
> >> return 0;
> >>
> >> }
> >>
> >> +static int drm_hdmi_check_mode(struct device *dev,
> >> + struct drm_display_mode *mode)
> >> +{
> >> + struct drm_hdmi_context *ctx = to_context(dev);
> >> +
> >> + return drm_hdmi_check_mode_ctx(ctx, mode);
> >> +}
> >> +
> >
> > nit: I don't think such wrapper is necessary.
> >
> > It seems to be easy enough to get from dev to ctx, so depending on the
> > amount of user of drm_hdmi_check_mode() it might be better to simply
> > change them to pass ctx instead of dev.
>
> This is a display_op that is defined to accept dev. It's changed later
> in the patchset to accept display, at which point the wrapper goes
> away.
Fair enough.
> > [snip]
> >
> >> @@ -403,19 +404,23 @@ static void vidi_subdrv_remove(struct
> >> drm_device
> >> *drm_dev, struct device *dev) /* TODO. */
> >>
> >> }
> >>
> >> -static int vidi_power_on(struct vidi_context *ctx, bool enable)
> >> +static int vidi_power_on(struct exynos_drm_manager *mgr, bool
> >> enable)
> >>
> >> {
> >>
> >> - struct exynos_drm_subdrv *subdrv = &ctx->subdrv;
> >> - struct device *dev = subdrv->dev;
> >> + struct vidi_context *ctx = mgr->ctx;
> >> +
> >> + DRM_DEBUG_KMS("%s\n", __FILE__);
> >> +
> >> + if (enable != false && enable != true)
> >> + return -EINVAL;
> >
> > Huh? What value would you expect a bool to have if not false or true?
> >
> > Anyway, this shouldn't really matter, as the check bellow assumes that
> > anything non-zero is true.
>
> This is pre-existing vidi code, I just moved it.
I don't see the hunk removing it from another location. Are you sure?
Best regards,
Tomasz
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
