| Message ID | 20220609234031.14803-5-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | Test plane alpha and zpos control | expand |
Quoting Laurent Pinchart (2022-06-10 00:40:25) > Centralize props value formatting in the AtomicRequest.add() function to > avoid having to call it manually through the code base. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > tests/kmstest.py | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/tests/kmstest.py b/tests/kmstest.py > index 14e28cd47fbd..2afaa513aa4d 100755 > --- a/tests/kmstest.py > +++ b/tests/kmstest.py > @@ -258,15 +258,20 @@ class AtomicRequest(pykms.AtomicReq): > self.__test = test > self.__props = {} > > + def __format_props(self, props): This is only validating the value arguements right, to ensure they are limited to 64 bit... > + return {k: v & ((1 << 64) - 1) for k, v in props.items()} > + Not this patch, but seems like 'add()' would benefit from some documentation here to show how to use it, what to pass etc. It's complex to read from just this context, but I can just about see that this patch is cleaning things up. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > def add(self, obj, *kwargs): > if obj.id not in self.__props: > self.__props[obj.id] = {} > - props = self.__props[obj.id] > + obj_props = self.__props[obj.id] > > if len(kwargs) == 1 and isinstance(kwargs[0], collections.abc.Mapping): > - props.update(kwargs[0]) > + props = self.__format_props(kwargs[0]) > elif len(kwargs) == 2: > - props[kwargs[0]] = kwargs[1] > + props = self.__format_props({ kwargs[0]: = kwargs[1] }) > + > + obj_props.update(props) > > super().add(obj, *kwargs) > > @@ -309,9 +314,6 @@ class KMSTest(object): > def __del__(self): > self.logger.close() > > - def __format_props(self, props): > - return {k: v & ((1 << 64) - 1) for k, v in props.items()} > - > def atomic_crtc_disable(self, crtc, sync=True): > req = AtomicRequest(self) > req.add(crtc, {'ACTIVE': 0, 'MODE_ID': 0}) > @@ -368,7 +370,7 @@ class KMSTest(object): > > def atomic_plane_set(self, plane, crtc, source, destination, fb, sync=False): > req = AtomicRequest(self) > - req.add(plane, self.__format_props({ > + req.add(plane, { > 'FB_ID': fb.id, > 'CRTC_ID': crtc.id, > 'SRC_X': int(source.left * 65536), > @@ -379,7 +381,7 @@ class KMSTest(object): > 'CRTC_Y': destination.top, > 'CRTC_W': destination.width, > 'CRTC_H': destination.height, > - })) > + }) > if sync: > return req.commit_sync() > else: > -- > Regards, > > Laurent Pinchart >
On Wed, Jun 29, 2022 at 04:36:49PM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart (2022-06-10 00:40:25) > > Centralize props value formatting in the AtomicRequest.add() function to > > avoid having to call it manually through the code base. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > tests/kmstest.py | 18 ++++++++++-------- > > 1 file changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/tests/kmstest.py b/tests/kmstest.py > > index 14e28cd47fbd..2afaa513aa4d 100755 > > --- a/tests/kmstest.py > > +++ b/tests/kmstest.py > > @@ -258,15 +258,20 @@ class AtomicRequest(pykms.AtomicReq): > > self.__test = test > > self.__props = {} > > > > + def __format_props(self, props): > > This is only validating the value arguements right, to ensure they are > limited to 64 bit... That's right, and it's mostly for negative values, that Python would otherwise handle incorrectly (if I recall correctly that's due to the bindings for the add() method mapping to the uint64_t, without automatic conversion for negative values). > > + return {k: v & ((1 << 64) - 1) for k, v in props.items()} > > + > > Not this patch, but seems like 'add()' would benefit from some > documentation here to show how to use it, what to pass etc. Good point. > It's complex to read from just this context, but I can just about see > that this patch is cleaning things up. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > def add(self, obj, *kwargs): > > if obj.id not in self.__props: > > self.__props[obj.id] = {} > > - props = self.__props[obj.id] > > + obj_props = self.__props[obj.id] > > > > if len(kwargs) == 1 and isinstance(kwargs[0], collections.abc.Mapping): > > - props.update(kwargs[0]) > > + props = self.__format_props(kwargs[0]) > > elif len(kwargs) == 2: > > - props[kwargs[0]] = kwargs[1] > > + props = self.__format_props({ kwargs[0]: = kwargs[1] }) > > + > > + obj_props.update(props) > > > > super().add(obj, *kwargs) > > > > @@ -309,9 +314,6 @@ class KMSTest(object): > > def __del__(self): > > self.logger.close() > > > > - def __format_props(self, props): > > - return {k: v & ((1 << 64) - 1) for k, v in props.items()} > > - > > def atomic_crtc_disable(self, crtc, sync=True): > > req = AtomicRequest(self) > > req.add(crtc, {'ACTIVE': 0, 'MODE_ID': 0}) > > @@ -368,7 +370,7 @@ class KMSTest(object): > > > > def atomic_plane_set(self, plane, crtc, source, destination, fb, sync=False): > > req = AtomicRequest(self) > > - req.add(plane, self.__format_props({ > > + req.add(plane, { > > 'FB_ID': fb.id, > > 'CRTC_ID': crtc.id, > > 'SRC_X': int(source.left * 65536), > > @@ -379,7 +381,7 @@ class KMSTest(object): > > 'CRTC_Y': destination.top, > > 'CRTC_W': destination.width, > > 'CRTC_H': destination.height, > > - })) > > + }) > > if sync: > > return req.commit_sync() > > else:
diff --git a/tests/kmstest.py b/tests/kmstest.py index 14e28cd47fbd..2afaa513aa4d 100755 --- a/tests/kmstest.py +++ b/tests/kmstest.py @@ -258,15 +258,20 @@ class AtomicRequest(pykms.AtomicReq): self.__test = test self.__props = {} + def __format_props(self, props): + return {k: v & ((1 << 64) - 1) for k, v in props.items()} + def add(self, obj, *kwargs): if obj.id not in self.__props: self.__props[obj.id] = {} - props = self.__props[obj.id] + obj_props = self.__props[obj.id] if len(kwargs) == 1 and isinstance(kwargs[0], collections.abc.Mapping): - props.update(kwargs[0]) + props = self.__format_props(kwargs[0]) elif len(kwargs) == 2: - props[kwargs[0]] = kwargs[1] + props = self.__format_props({ kwargs[0]: = kwargs[1] }) + + obj_props.update(props) super().add(obj, *kwargs) @@ -309,9 +314,6 @@ class KMSTest(object): def __del__(self): self.logger.close() - def __format_props(self, props): - return {k: v & ((1 << 64) - 1) for k, v in props.items()} - def atomic_crtc_disable(self, crtc, sync=True): req = AtomicRequest(self) req.add(crtc, {'ACTIVE': 0, 'MODE_ID': 0}) @@ -368,7 +370,7 @@ class KMSTest(object): def atomic_plane_set(self, plane, crtc, source, destination, fb, sync=False): req = AtomicRequest(self) - req.add(plane, self.__format_props({ + req.add(plane, { 'FB_ID': fb.id, 'CRTC_ID': crtc.id, 'SRC_X': int(source.left * 65536), @@ -379,7 +381,7 @@ class KMSTest(object): 'CRTC_Y': destination.top, 'CRTC_W': destination.width, 'CRTC_H': destination.height, - })) + }) if sync: return req.commit_sync() else:
Centralize props value formatting in the AtomicRequest.add() function to avoid having to call it manually through the code base. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- tests/kmstest.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)